Skip to content
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

Upgrade kdp to v0.15.0-beta.1 #1038

Merged

Conversation

maqiuyujoyce
Copy link
Collaborator

@maqiuyujoyce maqiuyujoyce commented Nov 28, 2023

Change description

Fixes b/309175495

  • Made a local copy of K8s core ResourceRequirements type to avoid adding new fields into ControllerResource and NamespacedControllerResource.
  • Replaced Watches() function with WatchesRawSource() function in controllers. Updated the parameter for Watches() function.
  • Added a non-nil cache to instantiate source.Kind (⚠️ Refactor source/handler/predicate packages to remove dep injection kubernetes-sigs/controller-runtime#2120).
  • Updated the test kubeclient in namecheck_test.go and upgrade_test.go.
  • Updated singleResourceClient and errorClient to match the latest interface of the controller-runtime client.
  • Removed the configuration of GoHeaderFilePath from deepcopy-gen and client-gen. The line of code turns out to be unnecessary.
  • Updated the client builders for the test clients.
  • Updated the registeration of the webhook server.
  • The following generated files are also changed:
    • The clientset/doc.go is removed (kubernetes/code-generator@8b5f587).
    • The instantiation of the resource and kind variables in the generated files under clientset is changed.

Tests you have done

  • Run make ready-pr to ensure this PR is ready for review.
  • [N/A] Perform necessary E2E testing for changed resources.
  • make test passed.
  • Deployed the controllers locally and successfully created a PubSubTopic.

@maqiuyujoyce maqiuyujoyce mentioned this pull request Nov 28, 2023
2 tasks
@maqiuyujoyce maqiuyujoyce force-pushed the 202311-update-kdp branch 2 times, most recently from a34cb8b to 1d8a442 Compare December 27, 2023 03:18
@maqiuyujoyce maqiuyujoyce changed the title [WIP] Upgrade kdp Upgrade kdp to v0.15.2 Dec 27, 2023
@justinsb
Copy link
Collaborator

justinsb commented Jan 2, 2024

LGTM except for the resources.claims change. I'm guessing this was introduced upstream in k8s and we're reusing the k8s type here. I'm also guessing we would have problems if someone actually set this new value (?). Maybe we want to define our own version of resources so we don't pick up the new "claims" field?

@maqiuyujoyce
Copy link
Collaborator Author

Hi @justinsb , thank you for reviewing!

I'm guessing this was introduced upstream in k8s and we're reusing the k8s type here.

Yes, claims field was introduced because the upstream type in K8s added it: kubernetes/api@v0.25.0...v0.27.2#diff-04e78d7bfddcb8b0b0fb2b950dbefcceb0af25bd827bfb41721f1280810a160aR2345.

I'm also guessing we would have problems if someone actually set this new value (?). Maybe we want to define our own version of resources so we don't pick up the new "claims" field?

I'm not familiar with this feature enough to determine if it'll be problematic or not off the top of my head, so agreed we should play it safe. I made a local copy of this type without the claims field.

@justinsb
Copy link
Collaborator

justinsb commented Jan 4, 2024

Thanks @maqiuyujoyce - lgtm with the embedding of the resource type!

/approve
/lgtm

Looks like it'll need a rebase on go.mod though

@google-oss-prow google-oss-prow bot added the lgtm label Jan 4, 2024
@google-oss-prow google-oss-prow bot removed the lgtm label Jan 4, 2024
@justinsb
Copy link
Collaborator

justinsb commented Jan 4, 2024

/approve
/lgtm

@google-oss-prow google-oss-prow bot added the lgtm label Jan 4, 2024
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: justinsb, maqiuyujoyce

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [justinsb,maqiuyujoyce]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@google-oss-prow google-oss-prow bot merged commit abdff8c into GoogleCloudPlatform:master Jan 4, 2024
6 checks passed
@maqiuyujoyce maqiuyujoyce changed the title Upgrade kdp to v0.15.2 Upgrade kdp to v0.15.0-beta.1 Jan 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants