Add RootSyncSet controller #3462
Conversation
mortent
requested review from
droot,
justinsb,
mengqiy,
natasha41575 and
yuwenma
as code owners
There was a problem hiding this comment.
I may be just out of the loop, but I am yet convinced on this concept at all. It's not clear to what problem it is solving. Can we get a use case description before we see code? I can't determine if this is the right approach before having a better understanding of the problem.
|
|
||
| // RootSyncSetSpec defines the desired state of RootSyncSet | ||
| type RootSyncSetSpec struct { | ||
| ClusterRefs []*ClusterRef `json:"clusterRefs,omitempty"` |
There was a problem hiding this comment.
Embedding these directly doesn't work at scale. Are we sure we want to introduce the concept of "Cluster"?
There was a problem hiding this comment.
I'd be fine with using "Target" instead (I think I'd personally prefer it), but I think we can iterate here.
There was a problem hiding this comment.
I guess this controller uses direct API access so cluster makes sense
There was a problem hiding this comment.
Yeah, it possible that this doesn't work at large scale. We can iterate on this when we get some experience with this.
| // RootSyncSetSpec defines the desired state of RootSyncSet | ||
| type RootSyncSetSpec struct { | ||
| ClusterRefs []*ClusterRef `json:"clusterRefs,omitempty"` | ||
| Template *RootSyncInfo `json:"template,omitempty"` |
There was a problem hiding this comment.
So we expect no variation between clusters?
There was a problem hiding this comment.
As I understand the plan, we're starting here and then going to figure out what variation we need.
There was a problem hiding this comment.
Yeah, no variation is a starting point. We know that for some (maybe many or all) cases we do need more than this.
mortent
force-pushed
the
AddRootSyncSet
branch
from
dde95cb
to
ccbf356
Compare
mortent
force-pushed
the
AddRootSyncSet
branch
from
ccbf356
to
47a21ea
Compare
|
/approve |
This adds the RootSyncSet controller to porch. This also copies code for token exchange to support workload identity from
pkg/registry/porch/witopkg/tokenexchange.This builds on #3456, so it must be merged after.