Skip to content
This repository has been archived by the owner on Oct 28, 2024. It is now read-only.

make CRD from v1beta1 to v1 #152

Closed
wants to merge 1 commit into from

Conversation

jichenjc
Copy link
Contributor

@jichenjc jichenjc commented Jul 1, 2021

What this PR does / why we need it:

make CRD become v1 of virtualcluster/config/crd/tenancy.x-k8s.io_clusterversions.yaml

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jul 1, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: jichenjc
To complete the pull request process, please assign charleszheng44 after the PR has been reviewed.
You can assign the PR to them by writing /assign @charleszheng44 in a comment when ready.

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

@k8s-ci-robot k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jul 1, 2021
@jichenjc
Copy link
Contributor Author

jichenjc commented Jul 1, 2021

/test pull-cluster-api-provider-nested-test

@jichenjc
Copy link
Contributor Author

jichenjc commented Jul 1, 2021

weird , my local test success ..

@Fei-Guo
Copy link

Fei-Guo commented Jul 1, 2021

/retest

@Fei-Guo
Copy link

Fei-Guo commented Jul 1, 2021

By the way, will this cause compatibility problem for old K8S version? When is the CRD API version promoted to v1?

@k8s-ci-robot
Copy link
Contributor

@jichenjc: The following test failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
pull-cluster-api-provider-nested-test c548343 link /test pull-cluster-api-provider-nested-test

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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.

@christopherhein
Copy link
Contributor

weird , my local test success ..

This could be a version difference w/ envtest and the binaries that are used to validate. @Fei-Guo 1.22 - https://kubernetes.io/docs/reference/using-api/deprecation-guide/#customresourcedefinition-v122

@jichenjc
Copy link
Contributor Author

jichenjc commented Jul 2, 2021

weird , my local test success ..

This could be a version difference w/ envtest and the binaries that are used to validate. @Fei-Guo 1.22 - https://kubernetes.io/docs/reference/using-api/deprecation-guide/#customresourcedefinition-v122

yeah, 1.22

Warning: apiextensions.k8s.io/v1beta1 CustomResourceDefinition is deprecated in v1.16+, unavailable in v1.22+; use apiextensions.k8s.io/v1 CustomResourceDefinition
customresourcedefinition.apiextensions.k8s.io/clusterversions.tenancy.x-k8s.io created

my local test succeed

Ran 0 of 0 Specs in 6.684 seconds
SUCCESS! -- 0 Passed | 0 Failed | 0 Pending | 0 Skipped
--- PASS: TestAPIs (6.68s)
PASS
ok      sigs.k8s.io/cluster-api-provider-nested/controlplane/nested/controllers (cached)
?       sigs.k8s.io/cluster-api-provider-nested/hack/boilerplate/test   [no test files]

@jichenjc
Copy link
Contributor Author

jichenjc commented Jul 2, 2021

/hold

my bad, I run test only on nested folder, not on virtual cluster folder
so need further test locally before submit

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 2, 2021
@jichenjc
Copy link
Contributor Author

jichenjc commented Jul 2, 2021

root@jjtest1:~# kubectl apply -f https://raw.githubusercontent.com/kubernetes-sigs/cluster-api-provider-nested/master/virtualcluster/config/crd/tenancy.x-k8s.io_clusterversions.yaml
kubectl apply -f https://raw.githubusercontent.com/kubernetes-sigs/cluster-api-provider-nested/master/virtualcluster/config/crd/tenancy.x-k8s.io_virtualclusters.yaml
error: unable to recognize "https://raw.githubusercontent.com/kubernetes-sigs/cluster-api-provider-nested/master/virtualcluster/config/crd/tenancy.x-k8s.io_clusterversions.yaml": no matches for kind "CustomResourceDefinition" in version "apiextensions.k8s.io/v1beta1"

FYI, use latest k8s cluster has problem so this is something need to be fixed before 1.22 GA

@christopherhein
Copy link
Contributor

@jichenjc are you still trying to get this fixed up?

@jichenjc
Copy link
Contributor Author

jichenjc commented Jul 5, 2021

@jichenjc are you still trying to get this fixed up?

yes, still working on , if any comment/help can be provided that will be helpful

@christopherhein
Copy link
Contributor

@jichenjc are you still trying to get this fixed up?

yes, still working on , if any comment/help can be provided that will be helpful

Sounds good, I'll try to take a look Tuesday if you haven't figured it out. Something that might help is #131 to help remove some of the CRDs now that we have the CAPN provisioner we might be able to wholly shift to CAPN and remove the native provisioner.

@jichenjc
Copy link
Contributor Author

jichenjc commented Jul 5, 2021

I narrowed down to here
https://github.com/kubernetes-sigs/cluster-api-provider-nested/blob/main/virtualcluster/pkg/controller/controllers/virtualcluster_controller_test.go#L48 but still not able to fix the problem

clusterVersion created here returns empty when Get function called, means this object not created successfully
I believe

  1. we should error out here first (can be done in further PR)
  2. I think the new CRD v1 is different to previous v1beta1 is multiple versions are added, somewhere need reflect but need help..

@jichenjc
Copy link
Contributor Author

jichenjc commented Jul 8, 2021

I think it's due to I manually updated this file, it should be regenerated though
there are some other places might need update as well, still checking but unforunately my env is broken recently

@jichenjc
Copy link
Contributor Author

jichenjc commented Jul 9, 2021

I gave up ..after fixed above issue , saw another one .. no idea what it happened though the code looks fine
maybe someone can help fix this later on

2021-07-09T14:45:24.685+0800    ERROR   controllers.VirtualCluster      fail to create virtualcluster   {"vc": "virtualcluster-samplewcghg", "retrytimes": 1, "error": "StatefulSet.apps \"etcd\" is invalid: spec.template.metadata.labels: Invalid value: map[string]string(nil): `selector` does not match template `labels`"}

@jichenjc jichenjc closed this Jul 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants