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

🌱 build(deps): bump github.com/operator-framework/operator-registry from v1.41 to v1.43 and github.com/operator-framework/api from v0.23.0 to v0.25.0 #3261

Conversation

perdasilva
Copy link
Collaborator

Description of the change:

Motivation for the change:

Architectural changes:

Testing remarks:

Reviewer Checklist

  • Implementation matches the proposed design, or proposal is updated to match implementation
  • Sufficient unit test coverage
  • Sufficient end-to-end test coverage
  • Bug fixes are accompanied by regression test(s)
  • e2e tests and flake fixes are accompanied evidence of flake testing, e.g. executing the test 100(0) times
  • tech debt/todo is accompanied by issue link(s) in comments in the surrounding code
  • Tests are comprehensible, e.g. Ginkgo DSL is being used appropriately
  • Docs updated or added to /doc
  • Commit messages sensible and descriptive
  • Tests marked as [FLAKE] are truly flaky and have an issue
  • Code is properly formatted

go.mod Outdated Show resolved Hide resolved
@dinhxuanvu
Copy link
Member

This PR needs a round of codegen run to update the CRDs.

@perdasilva
Copy link
Collaborator Author

Yeah, we need this first

@dinhxuanvu
Copy link
Member

Yeah, we need this first

You guys should plan a proper k8s rebase flow across all components to ensure a smooth operation and perhaps document it. k8s minor version bumps may come with some required changes. If it comes with a go version bump like this one (k8s 1.30 requires go 1.22), it may need even more changes.

@perdasilva
Copy link
Collaborator Author

You guys should plan a proper k8s rebase flow across all components to ensure a smooth operation and perhaps document it. k8s minor version bumps may come with some required changes. If it comes with a go version bump like this one (k8s 1.30 requires go 1.22), it may need even more changes.

I think this basically brings the trinity (api/registry/olm) to go 1.22 and k8s 1.30.
I had to do a lot of tinkering around the codegen bits because the scripts we were using were long depracated (and got removed in 1.30 XD)

@perdasilva
Copy link
Collaborator Author

looks like kube is picking up a couple of api violations:

CustomResourceDefinition.apiextensions.k8s.io "clusterserviceversions.operators.coreos.com" is invalid: [spec.validation.openAPIV3Schema.properties[spec].properties[install].properties[spec].properties[deployments].items.properties[spec].properties[template].properties[spec].properties[imagePullSecrets].items.properties[name].default: Required value: this property is in x-kubernetes-list-map-keys, so it must have a default or be a required property, spec.validation.openAPIV3Schema.properties[spec].properties[install].properties[spec].properties[deployments].items.properties[spec].properties[template].properties[spec].properties[hostAliases].items.properties[ip].default: Required value: this property is in x-kubernetes-list-map-keys, so it must have a default or be a required property]

Maybe we need to tune generation? or fix the apis?

@dinhxuanvu
Copy link
Member

looks like kube is picking up a couple of api violations:

CustomResourceDefinition.apiextensions.k8s.io "clusterserviceversions.operators.coreos.com" is invalid: [spec.validation.openAPIV3Schema.properties[spec].properties[install].properties[spec].properties[deployments].items.properties[spec].properties[template].properties[spec].properties[imagePullSecrets].items.properties[name].default: Required value: this property is in x-kubernetes-list-map-keys, so it must have a default or be a required property, spec.validation.openAPIV3Schema.properties[spec].properties[install].properties[spec].properties[deployments].items.properties[spec].properties[template].properties[spec].properties[hostAliases].items.properties[ip].default: Required value: this property is in x-kubernetes-list-map-keys, so it must have a default or be a required property]

Maybe we need to tune generation? or fix the apis?

This CRD has been generated years ago and I don’t remember when was the last it got a proper update. You may need to go back to the code where the api is constructed and check the comments to see if those k8s tags are still good.

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 21, 2024
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 22, 2024
@perdasilva
Copy link
Collaborator Author

looks like kube is picking up a couple of api violations:

CustomResourceDefinition.apiextensions.k8s.io "clusterserviceversions.operators.coreos.com" is invalid: [spec.validation.openAPIV3Schema.properties[spec].properties[install].properties[spec].properties[deployments].items.properties[spec].properties[template].properties[spec].properties[imagePullSecrets].items.properties[name].default: Required value: this property is in x-kubernetes-list-map-keys, so it must have a default or be a required property, spec.validation.openAPIV3Schema.properties[spec].properties[install].properties[spec].properties[deployments].items.properties[spec].properties[template].properties[spec].properties[hostAliases].items.properties[ip].default: Required value: this property is in x-kubernetes-list-map-keys, so it must have a default or be a required property]

Maybe we need to tune generation? or fix the apis?

This CRD has been generated years ago and I don’t remember when was the last it got a proper update. You may need to go back to the code where the api is constructed and check the comments to see if those k8s tags are still good.

Bad k8s version - seems running on v1.30+ fixes the issue

@perdasilva perdasilva force-pushed the perdasilva/bump/registry branch 2 times, most recently from 7ec34fd to b7f8d1f Compare May 22, 2024 07:53
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 23, 2024
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 24, 2024
@perdasilva perdasilva changed the title build(deps): bump github.com/operator-framework/operator-registry from v1.41 to v1.43 🌱 build(deps): bump github.com/operator-framework/operator-registry from v1.41 to v1.43 and github.com/operator-framework/api from v0.23.0 to v0.25.0 May 24, 2024
@perdasilva perdasilva force-pushed the perdasilva/bump/registry branch 2 times, most recently from 6232687 to dedb27b Compare May 24, 2024 13:37
…m v1.41 to v1.43.1

Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>
Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>
@perdasilva perdasilva force-pushed the perdasilva/bump/registry branch 4 times, most recently from 8cfe986 to dc58814 Compare May 25, 2024 13:14
Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>
@perdasilva
Copy link
Collaborator Author

closing in favor of #3282 - which solves the issue by pinning a couple of breaking dependencies

@perdasilva perdasilva closed this May 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants