Bump operator sdk v1.13.0 #376

AKamyshnikova · 2021-10-29T08:17:54Z

Q	A
Bug fix?	[]
New feature?	[x]
API breaks?	[]
Deprecations?	[]
Related tickets	fixes #X, partially #Y, mentioned in #Z
License	Apache 2.0

What's in this PR?

Use latest operator-sdk version.

Fixes #351

Additional context

v1.x.x of operator-sdk contains several breaking changes and require massive refactor of operator structure.
https://sdk.operatorframework.io/docs/building-operators/golang/migration/

Checklist

[x ] Implementation tested
Logging code meets the guideline
User guide and development docs updated (if needed)
Append changelog

To Do

Work is still in progress.
Multi-casskop is not finally switched yet.

AKamyshnikova · 2021-11-30T12:53:33Z

@cscetbon Hi! Could you upload newer version of casskop-build with v1.13.0 tag to orangeopensource? So I can update circleci settings as well.

cscetbon · 2021-12-01T04:05:16Z

@cscetbon Hi! Could you upload newer version of casskop-build with v1.13.0 tag to orangeopensource? So I can update circleci settings as well.

@AKamyshnikova Done ✅

AKamyshnikova · 2021-12-02T12:48:33Z

@cscetbon Thanks!
I faced issues with update-crds - it passes locally, but fails in CI...
Why we need such workaround for updating CRDs?
Seems that v1alpha1 folders has been renamed to v2 and it broke compatibility, so we need to leave v1alpha1 folders for backward compatibility. Was this considered?

cscetbon · 2021-12-02T14:32:22Z

That was for existing clusters. If you have to get rid of v1 do it

AKamyshnikova · 2021-12-06T08:08:21Z

@cscetbon
I just a little bit confused, I don't get any diff after running "make generate" locally. circleci/Dockerfile uses older controller-gen version, may be this is the problem. Sorry to bother, could you rebuild and upload casskop-build with v1.13.0 tag one more time? I've updated controller-gen version in latest commit.

cscetbon · 2021-12-07T13:42:10Z

@AKamyshnikova I just did it again 😉

Upgrade to operator-sdk 1.x version requires major structure refactoring [1]. Current commit updates file structure. [1] - https://sdk.operatorframework.io/docs/building-operators/golang/migration/ Signed-off-by: akamyshnikova <akamyshnikova@mirantis.com>

AKamyshnikova · 2021-12-22T12:35:21Z

@cscetbon This pull request is ready for review.

cscetbon · 2021-12-23T02:39:20Z

@AKamyshnikova it seems to be a big work. Not sure why kustomize is used but I started to review your PR, not done yet. But e2e tests are failing and you can fix them in the meantime ?

AKamyshnikova · 2021-12-23T07:39:38Z

@cscetbon Yeah, will fix this. Kustomize is part of new things with v1.x https://sdk.operatorframework.io/docs/building-operators/golang/migration/#what-is-new

cscetbon · 2021-12-24T17:46:46Z

@AKamyshnikova lmk when it's ready and I'll rerun the tests. You can try them locally using k3d like circle does

cscetbon

@AKamyshnikova a few comments. I like what you did. I would probably have done the renaming in a different PR to keep that one small but it's okay. thanks for your hard work !

.circleci/config.yml

Dockerfile

Makefile

api/v2/zz_generated.deepcopy.go

controllers/cassandrabackup/cassandrabackup_controller.go

controllers/cassandracluster/cassandracluster_controller.go

controllers/suite_test.go

multi-casskop/api/v2/cassandramulticluster_types.go

cscetbon · 2021-12-31T17:08:06Z

@AKamyshnikova trying to rerun tests on your branch I see that TestCassandraBackupAlreadyExists is failing. let me know when you think it's ready

AKamyshnikova · 2022-01-10T13:01:11Z

@cscetbon I've updated client-go to v.22.0 and controller-runtime to v0.9.0 - it seems to break unittests. I reverted to use v.0.19.3 and v0.6.5, I think update of these dependencies can be done in separate pull request as requires some additional work. Now tests are passing.

cscetbon · 2022-01-11T00:46:10Z

👏 @AKamyshnikova unit tests are fixed now. e2e tests are failing though because of an error with helm

go: unknown environment setting GO111MODULE=true
helm install ******* helm/cassandra-operator --set image.tag=trigger-integration
Error: INSTALLATION FAILED: failed to install CRD crds/*.yaml: no objects visited
make: *** [Makefile:279: kuttl-test-fix-arg] Error 1

AKamyshnikova · 2022-01-11T07:40:18Z

@cscetbon Seems I accidentally created file helm/cassandra-operator/crds/*.yaml :( I dropped it now.

cscetbon · 2022-01-11T15:42:30Z

Helm install doesn't fail anymore but e2e are failing. It seems it can't find the status key in the object for some reason

AKamyshnikova · 2022-01-12T11:32:02Z

@cscetbon One of changes is that ready is dropped https://sdk.operatorframework.io/docs/upgrading-sdk-version/v1.0.0/#removed-package-pkgready I missed that helm templates require update for this, fixed now.

cscetbon · 2022-01-12T21:58:03Z

e2e Tests ran again. Still missing status key in k8s object

AKamyshnikova · 2022-01-17T09:21:41Z

e2e Tests ran again. Still missing status key in k8s object

I run tests locally and don't get missing .status I'm not sure how to debug this for now.

AKamyshnikova · 2022-01-21T08:49:20Z

@cscetbon Hi! Could you allow running rest of the checks? I dropped +kubebuilder:subresource:status in latest patch set, may be this was a problem.

cscetbon · 2022-01-23T03:18:48Z

@cscetbon Hi! Could you allow running rest of the checks? I dropped +kubebuilder:subresource:status in latest patch set, may be this was a problem.

Better now. There is one remaining task failing but I'm not sure it's on you this time. I'm gonna review it. If in the meantime you think the failing test is on you don't hesitate to fix it 😉

AKamyshnikova · 2022-01-25T08:37:46Z

I checked and was some strange diff in two website/static/slides*.js files, hopefully fixed now.

cscetbon

Thanks for the hard work. A few comments and changes asked. Also do you know what would be the impact for a person who's already running the current version to move to this one ?

config/crd/kustomization.yaml

Dockerfile

cscetbon · 2022-01-27T15:43:52Z

config/crd/patches/cainjection_in_cassandraclusters.yaml

+metadata:
+  annotations:
+    cert-manager.io/inject-ca-from: $(CERTIFICATE_NAMESPACE)/$(CERTIFICATE_NAME)
+  name: cassandraclusters.db.orange.com


Do those files have any impact on an existing cluster ?

I will drop this, this is not enabled via kustomization.yaml so do not effect anything.

config/samples/db.orange.com_v2_cassandrarestore.yaml

docker/circleci/Dockerfile

main.go

multi-casskop/.gitignore

cscetbon · 2022-01-27T15:51:16Z

tools/create_dind_cluster.sh

@@ -25,7 +25,7 @@ kubectl create namespace cassandra-demo
 kubectl config set-context $(kubectl config current-context) --namespace=cassandra-demo

 echo "Create CRD"
-kubectl apply -f deploy/crds/db.orange.com_cassandraclusters_crd.yaml
+kubectl apply -f config/crd/bases/db.orange.com_cassandraclusters.yaml

 echo "configure helm"
 helm init


We don't use dind anymore. We use k3d, let's get rid of dind files

@AKamyshnikova it's still present 👆

AKamyshnikova · 2022-01-28T06:55:34Z

Thanks for the hard work. A few comments and changes asked. Also do you know what would be the impact for a person who's already running the current version to move to this one ?

While I was testing this pull request I've updated my test cluster (which use casskop v2.0.2 and has cassandra clusters) with this version and things work in the same way. So, from user perspective it would be just update to latest version, no additional steps will be needed.
Thanks for review! I will try to resolve comments soon.

Refactor pkg/apis and pkg/controller folders Signed-off-by: akamyshnikova <akamyshnikova@mirantis.com>

cscetbon · 2022-01-31T14:41:12Z

@AKamyshnikova please try to not force push anymore, it's hard for me to review your last changes. As soon as a review has started you shouldn't force push anymore, that way only the changes can be reviewed and not the whole thing every time.

cscetbon · 2022-01-31T15:19:13Z

Thanks for review! I will try to resolve comments soon.

Let me know when it's ready for review. Also I tried to rerun an old PR tests and the deploy works there. So there must be something wrong on your branch. Take a look at https://app.circleci.com/pipelines/github/Orange-OpenSource/casskop/1840/workflows/ef53e126-ec4b-453c-959d-2d3c20a7b52c/jobs/18369

AKamyshnikova · 2022-01-31T15:19:42Z

@cscetbon Sorry, I'm not used to github review process. Always has gerrit review for this :) Will keep that in mind.

AKamyshnikova · 2022-02-01T09:17:53Z

Yes, it is ready for review. Can you suggest how I can fix this branch?

fdehay · 2022-02-01T10:36:30Z

hello @AKamyshnikova ,
I fixed the circle-ci to github connection issue. It should be ok to run the tests now I think
Thanks for your hard work!

AKamyshnikova · 2022-02-01T11:32:23Z

@fdehay Thank you!

cscetbon · 2022-02-01T15:29:48Z

tools/create_dind_cluster.sh

@@ -25,7 +25,7 @@ kubectl create namespace cassandra-demo
 kubectl config set-context $(kubectl config current-context) --namespace=cassandra-demo

 echo "Create CRD"
-kubectl apply -f deploy/crds/db.orange.com_cassandraclusters_crd.yaml
+kubectl apply -f config/crd/bases/db.orange.com_cassandraclusters.yaml

 echo "configure helm"
 helm init


@AKamyshnikova it's still present 👆

cscetbon · 2022-02-02T13:45:40Z

Thanks @AKamyshnikova I'll take care of the few minor things I've seen. We'll try to test it more before making any new release and open issues if we find some

cscetbon · 2022-03-22T02:50:25Z

@AKamyshnikova any idea why in that PR you've renamed the CRD file ?

diff --git a/multi-casskop/deploy/crds/db.orange.com_multicasskops.yaml b/multi-casskop/config/crd/bases/multicluster_v1alpha1_cassandramulticluster_crd.yaml
similarity index 100%
rename from multi-casskop/deploy/crds/db.orange.com_multicasskops.yaml
rename to multi-casskop/config/crd/bases/multicluster_v1alpha1_cassandramulticluster_crd.yaml

You might want to create a PR to fix that PR. If yes, I moved the repo to https://github.com/cscetbon/casskop. You can be the 1st to create a PR there. I still need to create a PR here to redirect to my repo as I'm taking over from there.

AKamyshnikova · 2022-03-22T07:22:15Z

@cscetbon It is the name that is generated by controller-gen.
File with such name will always be created, to save old name we can only drop the one that are generated.
Something like this I did in PR with dump to 0.19
9c60518#diff-76ed074a9305c04054cdebb9e9aad2d818052b07091de1f20cad0bbac34ffb52R185

cscetbon · 2022-03-27T05:13:40Z

But why does it have v1alpha1 in its name ? I'm also fixing an issue in the backup/restore that was introduced by the upgrade of the sdk operator.

AKamyshnikova · 2022-03-28T08:55:37Z

I guess because when I start working on PR it was v1alpha1, I send PR with fixes cscetbon/casskop#29
Please point to issues you found and I will try to help.

cscetbon suggested changes Dec 24, 2021

View reviewed changes

This was referenced Dec 24, 2021

Kubernetes v1.22+ compatibility #385

Open

v2.0.3 INSTALLATION FAILED #386

Open

cscetbon suggested changes Jan 27, 2022

View reviewed changes

Bump operator-sdk v1.13.0

644b305

Refactor pkg/apis and pkg/controller folders Signed-off-by: akamyshnikova <akamyshnikova@mirantis.com>

cscetbon approved these changes Feb 2, 2022

View reviewed changes

cscetbon merged commit ea18f98 into Orange-OpenSource:master Feb 2, 2022

cscetbon mentioned this pull request Mar 9, 2022

V2.1.0 #398

Merged

AKamyshnikova mentioned this pull request Mar 28, 2022

Multicasskop fixes cscetbon/casskop#29

Merged

4 tasks

Bump operator sdk v1.13.0 #376

Bump operator sdk v1.13.0 #376

Conversation

AKamyshnikova commented Oct 29, 2021

What's in this PR?

Additional context

Checklist

To Do

AKamyshnikova commented Nov 30, 2021

cscetbon commented Dec 1, 2021

AKamyshnikova commented Dec 2, 2021

cscetbon commented Dec 2, 2021

AKamyshnikova commented Dec 6, 2021

cscetbon commented Dec 7, 2021

AKamyshnikova commented Dec 22, 2021

cscetbon commented Dec 23, 2021

AKamyshnikova commented Dec 23, 2021

cscetbon commented Dec 24, 2021

cscetbon left a comment

Choose a reason for hiding this comment

cscetbon commented Dec 31, 2021

AKamyshnikova commented Jan 10, 2022

cscetbon commented Jan 11, 2022

AKamyshnikova commented Jan 11, 2022

cscetbon commented Jan 11, 2022

AKamyshnikova commented Jan 12, 2022

cscetbon commented Jan 12, 2022

AKamyshnikova commented Jan 17, 2022

AKamyshnikova commented Jan 21, 2022

cscetbon commented Jan 23, 2022

AKamyshnikova commented Jan 25, 2022

cscetbon left a comment

Choose a reason for hiding this comment

cscetbon Jan 27, 2022

Choose a reason for hiding this comment

AKamyshnikova Jan 28, 2022

Choose a reason for hiding this comment

cscetbon Jan 27, 2022

Choose a reason for hiding this comment

cscetbon Feb 1, 2022

Choose a reason for hiding this comment

AKamyshnikova commented Jan 28, 2022

cscetbon commented Jan 31, 2022

cscetbon commented Jan 31, 2022

AKamyshnikova commented Jan 31, 2022

AKamyshnikova commented Feb 1, 2022

fdehay commented Feb 1, 2022 • edited Loading

AKamyshnikova commented Feb 1, 2022

cscetbon Feb 1, 2022

Choose a reason for hiding this comment

cscetbon commented Feb 2, 2022 • edited Loading

cscetbon commented Mar 22, 2022

AKamyshnikova commented Mar 22, 2022

cscetbon commented Mar 27, 2022

AKamyshnikova commented Mar 28, 2022

fdehay commented Feb 1, 2022 •

edited

Loading

cscetbon commented Feb 2, 2022 •

edited

Loading