[WIP] Sync GCP fleet memberships into KRM resources #3944
Conversation
|
|
||
| var ( | ||
| // GroupVersion is group version used to register these objects | ||
| GroupVersion = schema.GroupVersion{Group: "config.porch.kpt.dev", Version: "v1alpha1"} |
There was a problem hiding this comment.
If these are specifically Google fleet memberships, perhaps it should be in a google group?
An alternative is to create the concept of a "K8s endpoint" or "K8s hosting service". Any cluster could meet that requirements. This controller would then simply populate those objects; other controllers could populate similar objects from other sources.
There was a problem hiding this comment.
Yeah, I think it is worth considering if the group is correct here, or even if we should include this controller with others in kpt/porch. This controller, like the WorkloadIdentityBinding one, is very GCP-specific.
I did think about turning this into something like a K8s endpoint style resource, but I don't think we should. The information needed to connect to different types of clusters can differ, so I think we will have a hard time making this into a proper generic resource. Even within GCP there a quite a few different types of clusters available through the Hub API: https://pkg.go.dev/google.golang.org/api/gkehub/v1#MembershipEndpoint
For hooking these resources up to the PackageVariantSet controller, we should be able to use whatever resource represent a fleet membership (or something equivalent like K8s Endpoint) since we can use arbitrary objects. But some degree of consistency might be useful.
For connecting to different types of clusters, I think we need specific code for each "hosting service' that can consume the FleetMembership resource and something equivalent for other types. Each will know what it needs from the FleetMembership-like resource and how to use that information to connect to the cluster.
But open to discussing this.
There was a problem hiding this comment.
I could create a cloud-provider-gcp repo in Nephio. We would have this controller, plus any other integrations we do (for example the bootstrap controller integration).
| ) | ||
|
|
||
| type FleetSyncSpec struct { | ||
| ProjectIds []string `json:"projectIds"` |
There was a problem hiding this comment.
So, a fleet sync corresponds to a set of project IDs (fleet projects) from which we sync?
We would create a fleet sync to say we are interested in a particular set of fleets?
There was a problem hiding this comment.
Yeah, the current implementation requires a FleetSync CR for the controller to start syncing memberships into FleetMembership CRs from the fleets in the specified projects. It might be that we could just automatically start syncing from the same project as the current cluster, but I'm not sure if that is a good solution.
| } | ||
|
|
||
| type FleetMembershipStatus struct { | ||
| } |
There was a problem hiding this comment.
Ideally, we would have a way to reach these, to bootstrap in config sync, etc. So a client config of some kind. Sort of like RemoteRootSync. @henderiw is working on a bootstrap controller in Nephio that will enable us to install CS and a RootSync in newly created Cluster API-based clusters. I would want to be able to integrate that with this controller.
There was a problem hiding this comment.
Yeah, we can add more properties from the Fleet Membership API into the FleetMembership CRD. I've updated the PR to add Description, Labels, and State, but we can add more as needed. I'd rather start with less and add it as needed rather than syncing everything and possibly getting something wrong. There are a lot of fields available in the API.
We already have the different pieces needed to connect to clusters based on a membership, so getting that working on a controller should be pretty straightforward. I'm happy to help out on that.
|
Oof, sorry, that's not what I intended. I was trying to rebase. I guess I did it backwards. Let me try something...but I can undo this if necessary....I think. |
johnbelamaric
force-pushed
the
FleetSync
branch
from
ab2bad2
to
56ceed7
Compare
|
ok. fixed. let me try this another way |
|
Superseded by #4043 |
No description provided.