-
Notifications
You must be signed in to change notification settings - Fork 226
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support applying a package to a GKEHubMembership #3733
Conversation
7ed4617
to
7598a1b
Compare
|
||
var packageRevisions porchapi.PackageRevisionList | ||
labelFilter := client.MatchingLabels{ | ||
"kpt.dev/latest-revision": "true", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm planning to remove this label as described in #3672.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did this in #3734 as well. If we want this to be comparable, we should probably make it an int. We could also consider a "prior-revision" label, which might be easier to consume in practice. But anyway, we can compare using revision (string) for now, thanks for the heads-up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea is to make a couple of functions available through the Porch API that allows users to send in a slice of PackageRevision (and/or Unstructured) resources, and we will return the latest one based on our rules for the lifecycle and the revision.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That sounds good, though the simpler we can make the API, the more generally reusable/composable etc it will be. But yes, if we write the library, we might see opportunities to simplify the underlying API!
r.Client = mgr.GetClient() | ||
r.client = mgr.GetClient() | ||
|
||
uncachedClient, err := client.New(mgr.GetConfig(), client.Options{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe explain why this is needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added to #3734, which is the first commit split out!
Put a lot of the previous pieces together!
7598a1b
to
9475b69
Compare
return nil, err | ||
} | ||
|
||
host := fmt.Sprintf("https://connectgateway.googleapis.com/v1/projects/%d/locations/global/memberships/%s", projectInfo.ProjectNumber, membershipName) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I only see membershipName as part of the host
here. Does membership maps 1:1 to a cluster that is part of Hub ?
If yes
, then I have a follow up question: How do I discover all the memberships in a non-kcc world ?
I might block some time today for us to understand this machinery.
/cc @ChristopherFry
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess, hitting the GCP API using the workload identity of the config-controller
would one way to do this I guess.
Put a lot of the previous pieces together!