Boskos does not allow duplicate resource name even for different types

[ixdy]

Originally filed as kubernetes/test-infra#15054 by @amwat

resources:
- type: bar
  names: 
  - foo
- type: qwe
  names:
  - foo

results in {"error":"duplicated resource name: foo","level":"fatal","msg":"failed to create ranch!" }

This seems like something we should support?

/cc @krzyzacy @ixdy @sebastienvas

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[krzyzacy]

I think we need to handle Release and GetResource:

like https://github.com/kubernetes-sigs/boskos/blob/master/ranch/storage.go#L102:19 and https://github.com/kubernetes-sigs/boskos/blob/master/ranch/ranch.go#L279

[detiber]

/lifecycle frozen

[wking]

Pinning the links from the previous comment to a specific commit, because master has moved on:

$ git log --date=short --format='%ad %h %s' --first-parent | grep -B1 '2020-08'
2020-09-03 c0841a5 Merge pull request #39 from ixdy/go1.15.1
2020-08-18 984516e Merge pull request #38 from ixdy/boskos-janitor-logging-sinks

So:

• boskos/ranch/storage.go

  Line 102 in 984516e

  func (s *Storage) GetResource(name string) (*crds.ResourceObject, error) {

• boskos/ranch/ranch.go

  Line 279 in 984516e

  func (r *Ranch) Release(name, dest, owner string) error {

[wking]

I was poking around at this today, and I feel like the solution is to bump the ResourceObject CRD to v2, add a spec.name, and use metadata.name for an internal {type}-{name} slug or some such. That will still break if folks use something like:

resources:
- type: a
  names: 
  - b-c
- type: a-b
  names:
  - c

But I can't think of a simple guard for that, because any delimiters we could use could also be in the type or name. We could hash {type} {name} or some such, but that makes the resource names less recognizable. And with the consumed values moving into spec, we'd be free to easily migrate the metadata.name going forward, if we ended up running into problems with whatever approach we hit out of the gate.

However looking through the docs on CRD versions, I don't actually see any provision for mutating resource names. This might be a "drain the existing leases, delete all the existing ResourceObject, install the new CRD, and then push your config into new ResourceObjects" sort of thing. Or can we assume that the only thing touching these ResourceObjects is Boskos itself and wire the conversion into the reaper or some other process that is iterating over ResourceObjects?