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

makefile: containerize CRD generation and rename codegen and generate targets #2131

Merged
merged 2 commits into from
Feb 22, 2024

Conversation

willfindlay
Copy link
Contributor

Containerize the CRD generation workflow to resolve a version consistency issue we encountered. Rename the generate and codegen targets to disambiguate them.

@willfindlay willfindlay added the release-note/misc This PR makes changes that have no direct user impact. label Feb 21, 2024
@willfindlay willfindlay requested review from mtardy and a team as code owners February 21, 2024 21:24
Copy link

netlify bot commented Feb 21, 2024

Deploy Preview for tetragon ready!

Name Link
🔨 Latest commit 731c0f6
🔍 Latest deploy log https://app.netlify.com/sites/tetragon/deploys/65d66a271f6b090008a74583
😎 Deploy Preview https://deploy-preview-2131--tetragon.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Makefile Outdated Show resolved Hide resolved
@willfindlay willfindlay marked this pull request as draft February 22, 2024 15:09
We encountered some version-specific differences related to golang tooling which impact
the resulting CRD generation from make generate. To solve this problem, let's containerize
the CRD generation workflow so that we can get consistent output through consistent
tooling. while we're at it, rename the generate target to crds to disambiguate it from
protobuf code generation. We keep the old generate target around as an alias for backward
compatibility.

Signed-off-by: William Findlay <[email protected]>
Now that we have renamed the generate target to crds, it makes sense to also rename to
codegen target to make it more clear that it specifically refers to protobuf codegen. Just
as with make generate, we keep the old codegen target around to use as an alias for
backward compatbility. We also remove the original protogen dockerfile as it is no longer
needed. Instead, we can simply use the base cilium-builder image.

Signed-off-by: William Findlay <[email protected]>
@willfindlay willfindlay force-pushed the pr/willfindlay/containerized-generate branch from 731c0f6 to 6df735d Compare February 22, 2024 15:44
@willfindlay willfindlay marked this pull request as ready for review February 22, 2024 15:46
Copy link
Member

@mtardy mtardy left a comment

Choose a reason for hiding this comment

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

Thanks that's a really great patch, I was always confused between generate and codegen and this removes useless stuff and put stuff in the right place! Thank you

Makefile Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

I love that move!

@willfindlay willfindlay merged commit 5ac805f into main Feb 22, 2024
37 checks passed
@willfindlay willfindlay deleted the pr/willfindlay/containerized-generate branch February 22, 2024 16:38
This was referenced Feb 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants