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

Regenerate CRDs using controller-gen v0.9.2 #105

Merged
merged 2 commits into from Dec 4, 2022

Conversation

embano1
Copy link
Member

@embano1 embano1 commented Dec 2, 2022

Signed-off-by: Michael Gasch 15986659+embano1@users.noreply.github.com

Description of changes: When using the Kubernetes e2e-framework for testing a CRD setup function fails due to the newline in the two bases CRDs provided by this runtime.

The fix is to remove the newlines which should not cause any issues with ACK downstream dependencies, controllers, code-gen, etc.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Signed-off-by: Michael Gasch <15986659+embano1@users.noreply.github.com>
Copy link
Member

@a-hilaly a-hilaly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, thank you @embano1 !
Left one comment below

@@ -1,4 +1,3 @@

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The real real problem here is the controller-gen version we used to generate those files (v0.7.0) - Check line 7 in both files. This bug was fixed in v0.8.0 (PR kubernetes-sigs/controller-tools#517).
Can please regenerate these file using v0.9.2 ? it's the version we use for the generated controller now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, not sure I fully understand: I'm using aws-controllers-k8s/code-generator@20e82a5 with aws-controllers-k8s/runtime@007e038 (i.e. latest) to generate the files. From the tags those commits are way ahead of v0.[8|9].x?

Copy link
Member Author

@embano1 embano1 Dec 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And the weird thing is that the other service-related CRDs are created without the newline, i.e. only these two common files are somehow affected.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep the service related CRD are generated using controller-gen v0.9.2 - the common CRDs were generated long time ago using v0.7.0.
The thing is that CRD files are generated using controller-gen not ack-generate.

Script that build service related CRDs: https://github.com/aws-controllers-k8s/code-generator/blob/main/scripts/build-controller.sh#L196-L200
Script that build common CRDs: https://github.com/aws-controllers-k8s/runtime/blob/main/scripts/build-apis.sh#L20-L22

@a-hilaly
Copy link
Member

a-hilaly commented Dec 4, 2022

Just pushed 25f7576 that regenerates the CRD files with controller-gen v0.9.2
/approve

@a-hilaly a-hilaly changed the title Remove newline in crd bases Regenerate CRDs using controller-gen v0.9.2 Dec 4, 2022
@a-hilaly
Copy link
Member

a-hilaly commented Dec 4, 2022

/lgtm

@ack-bot ack-bot added the lgtm Indicates that a PR is ready to be merged. label Dec 4, 2022
@ack-bot
Copy link
Collaborator

ack-bot commented Dec 4, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: A-Hilaly, embano1

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:

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

@ack-bot ack-bot merged commit da127a6 into aws-controllers-k8s:main Dec 4, 2022
@embano1 embano1 deleted the crdbases branch December 5, 2022 07:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved lgtm Indicates that a PR is ready to be merged.
Projects
None yet
3 participants