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

Bump k8s.io dependencies #5097

Merged
merged 6 commits into from
Jul 4, 2022
Merged

Conversation

lucacome
Copy link
Contributor

@lucacome lucacome commented May 4, 2022

This PR:

  • updates Kubernetes libraries to v0.23.4 -> v0.24.2
  • updates controller-runtime v0.11.1 -> v0.11.2
  • updates controller-tools v0.7.0 -> v0.9.2
  • updates helm library v3.8.1 -> v3.9.0 (needed as [only this Helm is compatible with kube v0.24)
  • pins a bunch of open telemetry modules to older versions (as they seem to get bumped due to some transitive dependency of the newer helm libary, but clash with Kubernetes libraries- we'll be able to unpin once Use of outdated OpenTelemetry Go package kubernetes/kubernetes#106536 gets done)
  • changes informerResyncPeriod in unit test framework to not be less than the minimum resync period (see Slack thread) and bumps the wait time for test cache initial sync. The tests were failing on this - I am not exactly sure why as I don't see related changes in kube v0.24, but I assume it's because the wait time was less than the actual cache sync time.
Updates Kubernetes libraries to `v0.24.2`.

Signed-off-by: Luca Comellini luca.com@gmail.com

@jetstack-bot jetstack-bot added release-note-none Denotes a PR that doesn't merit a release note. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels May 4, 2022
@jetstack-bot
Copy link
Contributor

Hi @lucacome. Thanks for your PR.

I'm waiting for a cert-manager member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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/test-infra repository.

@jetstack-bot jetstack-bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label May 4, 2022
@lucacome lucacome changed the title Bump k8s.io dependencies to v0.24.0 Bump k8s.io dependencies May 4, 2022
@lucacome
Copy link
Contributor Author

lucacome commented May 4, 2022

/assign @JoshVanL

@SgtCoDFish
Copy link
Member

Thank you for raising this 😁

/ok-to-test

@jetstack-bot jetstack-bot added ok-to-test and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels May 5, 2022
@lucacome
Copy link
Contributor Author

lucacome commented May 5, 2022

I think this is needed first helm/helm#10928

@SgtCoDFish
Copy link
Member

Fair enough! There's no major rush, we don't have a release for at least a month 👍

@jetstack-bot jetstack-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 6, 2022
@jetstack-bot jetstack-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 6, 2022
@jetstack-bot jetstack-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 29, 2022
@irbekrm
Copy link
Contributor

irbekrm commented Jun 30, 2022

/milestone v1.9

@jetstack-bot jetstack-bot added this to the v1.9 milestone Jun 30, 2022
@irbekrm
Copy link
Contributor

irbekrm commented Jun 30, 2022

Hi @lucacome thanks again for working on this!

Do you think you will have time to rebase and update this PR? We're thinking to release the last 1.9 pre-release today/tomorrow and would be good to get this work in as well!
You might also need to run ./hack/update-bazel.sh to get the Bazel files updated

@lucacome
Copy link
Contributor Author

Hi @irbekrm I can rebase/update as soon as #5251 is merged

@irbekrm
Copy link
Contributor

irbekrm commented Jun 30, 2022

you should be able to update with ./hack/update-bazel.sh there shouldn't be a need for another command to update the Bazel files

@jetstack-bot jetstack-bot removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jun 30, 2022
@lucacome
Copy link
Contributor Author

/retest

@lucacome
Copy link
Contributor Author

 (22:28:59) ERROR: /bazel-scratch/.cache/bazel/_bazel_root/9a4ffe169a47b22ecc4990eba5f8384d/external/sh_helm_helm_v3/pkg/kube/BUILD.bazel:3:11: GoCompilePkg external/sh_helm_helm_v3/pkg/kube/kube.a failed: (Exit 1): builder failed: error executing command bazel-out/host/bin/external/go_sdk/builder compilepkg -sdk external/go_sdk -installsuffix linux_amd64 -src external/sh_helm_helm_v3/pkg/kube/client.go -src external/sh_helm_helm_v3/pkg/kube/config.go ... (remaining 104 argument(s) skipped)
Use --sandbox_debug to see verbose messages from the sandbox
external/sh_helm_helm_v3/pkg/kube/client.go:92:12: cannot use cmdutil.NewFactory(getter) (value of type "k8s.io/kubectl/pkg/cmd/util".Factory) as type Factory in struct literal:
	"k8s.io/kubectl/pkg/cmd/util".Factory does not implement Factory (wrong type for Validator method)
		have Validator(validationDirective string, verifier *"k8s.io/cli-runtime/pkg/resource".QueryParamVerifier) (validation.Schema, error)
		want Validator(validate bool) (validation.Schema, error) 

There seems to be at least one error related to helm. I'm not really sure how that can be fixed @irbekrm feel free to push to this PR if you have any ideas.

@irbekrm irbekrm assigned irbekrm and unassigned JoshVanL Jul 1, 2022
Signed-off-by: irbekrm <irbekrm@gmail.com>
Signed-off-by: irbekrm <irbekrm@gmail.com>
As the later version has a breaking change (bumps github.com/emicklei/go-restful -> github.com/emicklei/go-restful/v3)

Signed-off-by: irbekrm <irbekrm@gmail.com>
@irbekrm
Copy link
Contributor

irbekrm commented Jul 1, 2022

Building now succeeds, but he certificate signing request and gateway unit tests are still failing.

Comment on lines +242 to +255
github.com/containerd/containerd => github.com/containerd/containerd v1.5.10
github.com/miekg/dns v1.1.41 => github.com/miekg/dns v1.1.34
go.opentelemetry.io/contrib => go.opentelemetry.io/contrib v0.20.0
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc => go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.20.0
go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp => go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.20.0
go.opentelemetry.io/otel => go.opentelemetry.io/otel v0.20.0
go.opentelemetry.io/otel/exporters/otlp => go.opentelemetry.io/otel/exporters/otlp v0.20.0
go.opentelemetry.io/otel/metric => go.opentelemetry.io/otel/metric v0.20.0
go.opentelemetry.io/otel/oteltest => go.opentelemetry.io/otel/oteltest v0.20.0
go.opentelemetry.io/otel/sdk => go.opentelemetry.io/otel/sdk v0.20.0
go.opentelemetry.io/otel/sdk/export/metric => go.opentelemetry.io/otel/sdk/export/metric v0.20.0
go.opentelemetry.io/otel/sdk/metric => go.opentelemetry.io/otel/sdk/metric v0.20.0
go.opentelemetry.io/otel/trace => go.opentelemetry.io/otel/trace v0.20.0
go.opentelemetry.io/proto/otlp => go.opentelemetry.io/proto/otlp v0.7.0
Copy link
Contributor

Choose a reason for hiding this comment

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

These are needed till kubernetes/kubernetes#106536 gets fixed

As minimum resync period in client-go is 1s. Also makes sure that the tests don't sleep for 'too long'.

Signed-off-by: irbekrm <irbekrm@gmail.com>
Signed-off-by: irbekrm <irbekrm@gmail.com>
@irbekrm
Copy link
Contributor

irbekrm commented Jul 4, 2022

/test pull-cert-manager-e2e-feature-gates-disabled
/test pull-cert-manager-e2e-issuers-venafi-cloud
/test pull-cert-manager-e2e-v1-20
/test pull-cert-manager-e2e-v1-21
/test pull-cert-manager-e2e-v1-22
/test pull-cert-manager-e2e-v1-23
/test pull-cert-manager-issuers-venafi-tpp

@cert-manager cert-manager deleted a comment from jetstack-bot Jul 4, 2022
@jetstack-bot jetstack-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Jul 4, 2022
@cert-manager cert-manager deleted a comment from jetstack-bot Jul 4, 2022
@irbekrm
Copy link
Contributor

irbekrm commented Jul 4, 2022

This should now be ready for another lgtm (I cannot do that as I've now also contributed)

const informerResyncPeriod = time.Millisecond * 10
const informerResyncPeriod = time.Second
Copy link
Member

Choose a reason for hiding this comment

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

Comment on lines -378 to +379
KUBEBUILDER_TOOLS_linux_amd64_SHA256SUM=25daf3c5d7e8b63ea933e11cd6ca157868d71a12885aba97d1e7e1a15510713e
KUBEBUILDER_TOOLS_darwin_amd64_SHA256SUM=bb27efb1d2ee43749475293408fc80b923324ab876e5da54e58594bbe2969c42
KUBEBUILDER_TOOLS_linux_amd64_SHA256SUM=6d9f0a6ab0119c5060799b4b8cbd0a030562da70b7ad4125c218eaf028c6cc28
KUBEBUILDER_TOOLS_darwin_amd64_SHA256SUM=3367987e2b40dadb5081a92a59d82664bee923eeeea77017ec88daf735e26cae
Copy link
Member

Choose a reason for hiding this comment

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

note: just spotted that they've added darwin-arm64 support so we can update to use that and remove our rosetta-based workaround. I'll raise a separate PR for that, don't think it needs to go in this one!

Copy link
Member

@SgtCoDFish SgtCoDFish left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve
/hold

Adding a hold in case anyone else wants to take a look. This seems totally reasonable but I've not dug deeply into the version upgrades or anything!

Thank you to everyone who's been involved in this, looks like it was quite a bit of work!

@jetstack-bot jetstack-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 4, 2022
@jetstack-bot jetstack-bot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jul 4, 2022
Copy link
Member

@jakexks jakexks left a comment

Choose a reason for hiding this comment

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

/unhold

@jetstack-bot jetstack-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 4, 2022
@jetstack-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jakexks, lucacome, SgtCoDFish

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

@jetstack-bot jetstack-bot merged commit 5a4e765 into cert-manager:master Jul 4, 2022
@lucacome lucacome deleted the bump-k8s-deps branch July 7, 2022 21:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/deploy Indicates a PR modifies deployment configuration area/testing Issues relating to testing dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm Indicates that a PR is ready to be merged. ok-to-test release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants