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

Upgrade controller runtime 0.8.3 and K8s 1.20 #1207

Merged

Conversation

cheyang
Copy link
Collaborator

@cheyang cheyang commented Nov 27, 2021

Ⅰ. Describe what this PR does

  1. Upgrade K8s to 1.20.10
  2. Upgrade controller-runtime to 0.8.3 and controller-gen to 0.7.0, and adopt the breaking change of them
    • The interface for client.Get, Client.Create, Clident.Update is changed from runtime.Object to client.Object
    • The parameter of function Reconcile is changed from req ctrl.Request to ctx context.Context, req ctrl.Request
    • some test function is deprecated.
  3. Upgrade API version
    • CustomResourceDefinition from apiextensions.k8s.io/vebeta1 to apiextensions.k8s.io/v1
    • CSIDriver from storage.k8s.io/v1beta1 to storage.k8s.io/v1
    • MutatingWebhookConfiguration from admissionregistration.k8s.io/v1beta1 to admissionregistration.k8s.io/v1

Ⅱ. Does this pull request fix one issue?

fixes #1208

Ⅲ. List the added test cases (unit test/integration test) if any, please explain if no tests are needed.

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

Signed-off-by: cheyang <cheyang@163.com>
Signed-off-by: cheyang <cheyang@163.com>
Signed-off-by: cheyang <cheyang@163.com>
Signed-off-by: cheyang <cheyang@163.com>
Signed-off-by: cheyang <cheyang@163.com>
Signed-off-by: cheyang <cheyang@163.com>
Signed-off-by: cheyang <cheyang@163.com>
Signed-off-by: cheyang <cheyang@163.com>
Signed-off-by: cheyang <cheyang@163.com>
Signed-off-by: cheyang <cheyang@163.com>
Signed-off-by: cheyang <cheyang@163.com>
Signed-off-by: cheyang <cheyang@163.com>
@cheyang cheyang force-pushed the upgrade_controller_runtime_0.7.0 branch 3 times, most recently from 73456bd to c0f3c89 Compare November 27, 2021 13:02
Signed-off-by: cheyang <cheyang@163.com>
Signed-off-by: cheyang <cheyang@163.com>
Signed-off-by: cheyang <cheyang@163.com>
Signed-off-by: cheyang <cheyang@163.com>
@codecov
Copy link

codecov bot commented Nov 27, 2021

Codecov Report

Merging #1207 (8312dc8) into master (2c78e52) will increase coverage by 0.39%.
The diff coverage is 83.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1207      +/-   ##
==========================================
+ Coverage   66.30%   66.69%   +0.39%     
==========================================
  Files         207      207              
  Lines       11152    11156       +4     
==========================================
+ Hits         7394     7441      +47     
+ Misses       2964     2916      -48     
- Partials      794      799       +5     
Impacted Files Coverage Δ
pkg/utils/kubeclient/metadata.go 77.77% <ø> (ø)
pkg/utils/dataset/lifecycle/schedule.go 62.60% <80.00%> (+37.39%) ⬆️
pkg/webhook/certificate_builder.go 60.97% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2c78e52...8312dc8. Read the comment docs.

Signed-off-by: cheyang <cheyang@163.com>
Signed-off-by: cheyang <cheyang@163.com>
Signed-off-by: cheyang <cheyang@163.com>
Signed-off-by: cheyang <cheyang@163.com>
Signed-off-by: cheyang <cheyang@163.com>
@@ -1,143 +1,146 @@

---
apiVersion: apiextensions.k8s.io/v1beta1
apiVersion: apiextensions.k8s.io/v1
Copy link
Member

Choose a reason for hiding this comment

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

apiextensions.k8s.io/v1 is supported since v1.16, what about k8s v1.14/v1.15 ?

k8s.io/klog v1.0.0
k8s.io/klog/v2 v2.9.0 // indirect
k8s.io/kube-openapi v0.0.0-20201113171705-d219536bb9fd
k8s.io/kubernetes v1.20.10
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we not use Kubernetes pkg directly ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm using bash k8s-mod.sh v1.20.10 to upgrade K8s version. This script is from K8s and using the exact same version in K8s repo.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Now it's fixed. See the latest commit.

Copy link
Contributor

Choose a reason for hiding this comment

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

awesome !
Because if Fluid uses the kubernetes package directly, a project that references Fluid must also maintain replace in go.mod,
This is not an official recommended approach, kubernetes/kubernetes#90358 (comment)

Copy link
Collaborator Author

@cheyang cheyang Nov 29, 2021

Choose a reason for hiding this comment

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

I was using same useful library, and I am afraid it's Inevitable.

I was using the solution in kubernetes/kubernetes#79384. It works as a charm.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, I got it

Signed-off-by: cheyang <cheyang@163.com>
Signed-off-by: cheyang <cheyang@163.com>
@xieydd
Copy link
Member

xieydd commented Nov 28, 2021

Can you describe the impact of the version update and why it was updated in issue #1208 ?

@cheyang
Copy link
Collaborator Author

cheyang commented Nov 29, 2021

Can you describe the impact of the version update and why it was updated in issue #1208 ?

The purpose is to match K8s 1.22. As you know a lot of APIs are removed since 1.22, including CSI, CRD, webhook. We have to upgrade the K8s version, and during the upgrade we found the controller-runtime has to be upgraded also.

Signed-off-by: cheyang <cheyang@163.com>
@xieydd
Copy link
Member

xieydd commented Nov 29, 2021

For User, if your kubernetes version under 1.16.0, can only use fluid berfore this pr, right?
And i notice we update k8s to 1.20.10, not 1.22.

@cheyang
Copy link
Collaborator Author

cheyang commented Nov 29, 2021

For User, if your kubernetes version under 1.16.0, can only use fluid berfore this pr, right? And i notice we update k8s to 1.20.10, not 1.22.

It can support the K8s version whose version >= 1.16. Actually, K8s 1.14's EOL date is 2019-12-11, about 2 years ago. But I plan to provide limited support in helm chart, for example in the version 1.14, we can install CRD with v1beta1, while webhook is not supported.


https://kubernetes.io/releases/patch-releases/#non-active-branch-history

Copy link
Contributor

@allenhaozi allenhaozi left a comment

Choose a reason for hiding this comment

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

LGTM

@cheyang
Copy link
Collaborator Author

cheyang commented Nov 29, 2021

LGTM

thanks.

@zwwhdls
Copy link
Member

zwwhdls commented Nov 29, 2021

LGTM

@cheyang cheyang merged commit 1a6ab9d into fluid-cloudnative:master Nov 29, 2021
@cheyang cheyang deleted the upgrade_controller_runtime_0.7.0 branch April 18, 2022 09:26
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.

[FEATURES] Upgrade controller runtime and K8s version
4 participants