Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

HIVE-2558: Drop codegen k8s.io/code-generator #2342

Closed
wants to merge 1 commit into from

Conversation

celebdor
Copy link
Contributor

@celebdor celebdor commented Jul 4, 2024

generate-groups.sh was deprecated in k8s.io/code-generator 1.29 and deleted in 1.30. Instead, the usage was replaced with:

  • kube_codegen.sh library's kube::codegen::gen_helpers

The issue with the above is:

  • It doesn't allow to specify just running deepcopy and instead adds as well code conversion and defaulter code generation.
  • While deepcopy-gen and conversion-gen can deal well with the multiple input paths that we have, conversion-gen wants to work at the package level, which means it ends up wrongly processing the vendored k8s deepcopy directives. This in turn means that we are forced to use the kubernetes header files to respect their copyright, but ends up adding the kubernetes copyright to openshift generated code.
  • gen_helpers enforces usage of --output-base matching the import name, which means that we had to workaround with a symlink of ${REPO_ROOT}/github.com/openshift/ to "${REPO_ROOT}..".
  • vendored generated deepcopy got deleted and make vendor had to pick up the tab.

generate-groups.sh was deprecated in k8s.io/code-generator 1.29 and deleted in
1.30. Instead, the usage was replaced with:

- kube_codegen.sh library's kube::codegen::gen_helpers

The issue with the above is:

- It doesn't allow to specify just running deepcopy and instead adds as
  well code conversion and defaulter code generation.
- While deepcopy-gen and conversion-gen can deal well with the multiple
  input paths that we have, conversion-gen wants to work at the package
  level, which means it ends up wrongly processing the vendored k8s
  deepcopy directives. This in turn means that we are forced to use the
  kubernetes header files to respect their copyright, but ends up adding
  the kubernetes copyright to openshift generated code.
- gen_helpers enforces usage of --output-base matching the import name,
  which means that we had to workaround with a symlink of
  ${REPO_ROOT}/github.com/openshift/ to "${REPO_ROOT}..".
- vendored generated deepcopy got deleted and `make vendor` had to pick
  up the tab.

Signed-off-by: Antoni Segura Puimedon <antoni@redhat.com>
@openshift-ci-robot
Copy link

openshift-ci-robot commented Jul 4, 2024

@celebdor: This pull request references HIVE-2558 which is a valid jira issue.

In response to this:

generate-groups.sh was deprecated in k8s.io/code-generator 1.29 and deleted in 1.30. Instead, the usage was replaced with:

  • kube_codegen.sh library's kube::codegen::gen_helpers

The issue with the above is:

  • It doesn't allow to specify just running deepcopy and instead adds as well code conversion and defaulter code generation.
  • While deepcopy-gen and conversion-gen can deal well with the multiple input paths that we have, conversion-gen wants to work at the package level, which means it ends up wrongly processing the vendored k8s deepcopy directives. This in turn means that we are forced to use the kubernetes header files to respect their copyright, but ends up adding the kubernetes copyright to openshift generated code.
  • gen_helpers enforces usage of --output-base matching the import name, which means that we had to workaround with a symlink of ${REPO_ROOT}/github.com/openshift/ to "${REPO_ROOT}..".
  • vendored generated deepcopy got deleted and make vendor had to pick up the tab.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jul 4, 2024
@openshift-ci openshift-ci bot requested review from dlom and suhanime July 4, 2024 18:26
Copy link
Contributor

openshift-ci bot commented Jul 4, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: celebdor
Once this PR has been reviewed and has the lgtm label, please assign 2uasimojo for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found 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

Copy link
Contributor

openshift-ci bot commented Jul 4, 2024

@celebdor: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/periodic-images b60141f link true /test periodic-images
ci/prow/security b60141f link true /test security
ci/prow/verify b60141f link true /test verify

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@openshift-bot
Copy link

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

@openshift-ci openshift-ci bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 3, 2024
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 3, 2024
@openshift-merge-robot
Copy link
Contributor

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

${SED_CMD} -i 's,^exec \(.*/generate-internal-groups.sh\),bash \1,g' ${CODEGEN_PKG}/generate-groups.sh
# ...but we have to put it back, or `verify` will puke.
trap "git checkout ${CODEGEN_PKG}/generate-groups.sh" EXIT
GO111MODULE=on go install k8s.io/code-generator/cmd/deepcopy-gen@release-1.29
Copy link
Member

@2uasimojo 2uasimojo Oct 3, 2024

Choose a reason for hiding this comment

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

I ran into the same failure with go install <web URI> in #2481

Can we try:

  • Make sure we have code-generator vendored at the appropriate version in go.mod, and naked-imported in pkg/dependencymagnet/go.doc
  • Use go install $(shell realpath vendor/k8s.io/code-generator/cmd/deepcopy-gen) here to build the tool from the vendor dir

?

@2uasimojo
Copy link
Member

/remove-lifecycle stale

@celebdor lmk if you want me to pick this up.

@openshift-ci openshift-ci bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 3, 2024
@openshift-bot
Copy link

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

@openshift-ci openshift-ci bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 2, 2025
@2uasimojo
Copy link
Member

Looking at the current tip of master, this seems to have already been done elsewhere.

@2uasimojo 2uasimojo closed this Jan 3, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants