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

Update controller runtime to v0.9.5 and k8s dependencies to v0.21.3 #540

Closed
wants to merge 6 commits into from

Conversation

amisevsk
Copy link
Collaborator

@amisevsk amisevsk commented Aug 4, 2021

What does this PR do?

Update controller-runtime to v0.9.5 and associated dependencies to match (mainly, k8s deps to v0.21.3)

Commits walk through required changes step-by-step -- we're going through 3 versions of breaking changes so there's a lot of scattered updates needed.

What issues does this PR fix or reference?

Fixes #516
Fixes #543

Is it tested? How?

Should test controller thoroughly; I've done basic testing and found no issues.

PR Checklist

  • E2E tests pass (when PR is ready, comment /test v7-devworkspaces-operator-e2e, v7-devworkspace-happy-path to trigger)
    • v7-devworkspaces-operator-e2e: DevWorkspace e2e test
    • v7-devworkspace-happy-path: DevWorkspace e2e test

@openshift-ci openshift-ci bot added the approved label Aug 4, 2021
@amisevsk amisevsk force-pushed the update-controller-runtime branch from a2d20a7 to b24fa5e Compare August 4, 2021 15:12
@amisevsk
Copy link
Collaborator Author

amisevsk commented Aug 4, 2021

One note about the updated controller-runtime dependency: the latest controller-runtime releases suggest updating to go 1.16, which we currently cannot do since no 1.16 UBI8 images appear to be available at this time. This isn't a huge problem, but some internal packages in controller-runtime make use of the io package, which causes go mod tidy to fail on go <1.15.10. See issue golang/go#44557 for details.

I've tried to sync everything to match go 1.15.13, which is used in our current UBI image, but we should update to 1.16 once that image becomes available.

Copy link
Member

@sleshchenko sleshchenko left a comment

Choose a reason for hiding this comment

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

LGTM

@openshift-ci openshift-ci bot added the lgtm label Aug 11, 2021
@sleshchenko sleshchenko force-pushed the update-controller-runtime branch from 3274700 to f605d9e Compare August 11, 2021 13:17
@openshift-ci openshift-ci bot removed the lgtm label Aug 11, 2021
@sleshchenko
Copy link
Member

rebased against main to fix PR checks

@sleshchenko
Copy link
Member

/test v7-devworkspaces-operator-e2e, v7-devworkspace-happy-path

Copy link
Contributor

@JPinkney JPinkney left a comment

Choose a reason for hiding this comment

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

LGTM

@amisevsk
Copy link
Collaborator Author

/test v7-devworkspaces-operator-e2e, v7-devworkspace-happy-path

@amisevsk
Copy link
Collaborator Author

Great news: turns out prow CI is using some older version of Go and so checks will always fail due to #540 (comment)

@amisevsk amisevsk force-pushed the update-controller-runtime branch from f605d9e to 2f0afec Compare August 12, 2021 18:04
@openshift-ci openshift-ci bot removed the lgtm label Aug 12, 2021
@amisevsk
Copy link
Collaborator Author

Added to PR:

  • Update devfile/api dependency to match k8s v0.21.3 change (upgrade k8s api to v0.21.3 api#565)
  • Update webhooks config to use v1 webhooks instead of v1beta1 (and also networking.k8s.io/v1 Ingresses)

@amisevsk amisevsk force-pushed the update-controller-runtime branch 2 times, most recently from 0639367 to 3d31311 Compare August 12, 2021 20:40
Prep code for supporting k8s 1.21:

* Add context.Context parameter to Reconcile() functions [1]
* Add RBAC get/create/update coordination.k8s.io/v1 Leases (replacing a
  configmap for leader election locks) [2]
* Adapt event handler code to reflect simplification [3]
* Adapt to apimachinery/pkg/runtime Log deprecation (replaced with
  pkg/client Log) [4]
* client.Object is preferred in favor of runtime.Object (v0.7.0 release) [5]
* Use admission/v1 instead of v1beta1 for webhook requests [6]

[1] - kubernetes-sigs/controller-runtime#1054
[2] - kubernetes-sigs/controller-runtime#1144
[3] - kubernetes-sigs/controller-runtime#1119
[4] - kubernetes-sigs/controller-runtime#1105
[5] - kubernetes-sigs/controller-runtime#898
      kubernetes-sigs/controller-runtime#1118
[6] - kubernetes-sigs/controller-runtime#1284

kubernetes-sigs/controller-runtime@a32b29d
Signed-off-by: Angel Misevski <[email protected]>
Update sigs.k8s.io/controller-runtime to version v0.9.5, and match other
dependencies to those used by controller-runtime, notably:

* k8s.io/api, k8s.io/apimachinery, k8s.io/client-go updated to v0.21.3
* Go version should be upgraded to 1.16 but isn't since there isn't a Go
  1.16 UBI image we can use as builder.

Signed-off-by: Angel Misevski <[email protected]>
extensions/v1beta1 Ingresses are deprecated as of k8s v1.14 and will be
removed in v1.22

Signed-off-by: Angel Misevski <[email protected]>
Update the devfile/api dependency to the latest commit, to match the
update to k8s v0.21.3.

Signed-off-by: Angel Misevski <[email protected]>
Required as v1beta1 webhooks will be unavailable soon.

Also update webhooks configs to apply to v1 Ingresses on k8s
(networking.k8s.io API group)

Signed-off-by: Angel Misevski <[email protected]>
@amisevsk amisevsk force-pushed the update-controller-runtime branch from 3d31311 to 87e377d Compare August 12, 2021 21:41
@amisevsk
Copy link
Collaborator Author

/test v7-devworkspaces-operator-e2e, v7-devworkspace-happy-path

@sleshchenko
Copy link
Member

/test v7-devworkspaces-operator-e2e

@openshift-ci
Copy link

openshift-ci bot commented Aug 16, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: amisevsk, JPinkney, metlos, sleshchenko

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:
  • OWNERS [JPinkney,amisevsk,sleshchenko]

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

@metlos
Copy link
Contributor

metlos commented Aug 16, 2021

Actually, I would prefer if there were 2 PRs, if possible - one for ingress v1 and the other for the controller runtime upgrade. Upgrading the controller runtime in che-operator (which depends on DWO) is going to be a much harder task and we should not block progress on v1beta1 removal there by requiring a simulatenous upgrade of controller runtime.

@openshift-ci
Copy link

openshift-ci bot commented Aug 26, 2021

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

@openshift-ci
Copy link

openshift-ci bot commented Sep 1, 2021

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

Test name Commit Details Rerun command
ci/prow/v7-devworkspace-happy-path 87e377d link /test v7-devworkspace-happy-path
ci/prow/v7-devworkspaces-operator-e2e 87e377d link /test v7-devworkspaces-operator-e2e
ci/prow/v8-images 87e377d link /test v8-images

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/test-infra repository. I understand the commands that are listed here.

@sleshchenko
Copy link
Member

Controller runtime is upgraded in #567
K8s API and dropping usage of beta APIs are part of #556

@sleshchenko sleshchenko closed this Sep 2, 2021
@amisevsk amisevsk deleted the update-controller-runtime branch February 8, 2023 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Get rid of usage deprecated APIs v1beta1 webhooks, Ingress Update K8s API to newest
4 participants