upgrade operator-sdk to v0.9.0 #88

jsanda · 2019-08-05T18:23:37Z

This PR is for #75.

I want to make a note of a breaking change. metrics.CreateMetricsService(ctx, metricsPort) was removed and replaced with metrics.CreateMetricsService(ctx, cfg, servicePorts). See operator-framework/operator-sdk#1560 for details.

jsanda · 2019-08-05T18:35:23Z

@allamand CircleCI is still using v0.7.0 of operator-sdk, but I updated Gopkg.toml. Do you know what the issue is? Looks like we might need make force-get-deps.

allamand · 2019-08-05T19:05:06Z

I think I need to rebuild image for circleCI with new version

jsanda · 2019-08-05T19:33:42Z

I think I need to rebuild image for circleCI with new version

Is that something that I can do?

allamand · 2019-08-05T20:03:17Z

It may be something like make docker-dev-ci or similar but you won’t have right to push it on the orange open source docker hub But you should try it on your own repo to go further Sébastien Le 5 août 2019 à 21:33, John Sanda <notifications@github.com<mailto:notifications@github.com>> a écrit : I think I need to rebuild image for circleCI with new version Is that something that I can do? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#88>, or mute the thread<https://github.com/notifications/unsubscribe-auth/ACEKVYZ5HJWO4JNZRIB7UTTQDB6BPANCNFSM4IJNXGWA>.

…

_________________________________________________________________________________________________________________________ Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration, Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci. This message and its attachments may contain confidential or privileged information that may be protected by law; they should not be distributed, used or copied without authorisation. If you have received this email in error, please notify the sender and delete this message and its attachments. As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified. Thank you.

jsanda · 2019-08-05T20:20:22Z

I did the following before committing/pushing,

$ make clean
$ make get-deps
$ make build
$ make unit-test

Unit tests passed. @allamand are you suggesting I do something else?

allamand · 2019-08-06T06:28:32Z

I think to reproduce on local what is doing on circleCI you must use

Make docker-build

But first you need to build new image with new operator dsl version with

Make build-ci-image

jsanda · 2019-08-06T18:34:28Z

@allamand I think I finally got it sorted out. Here is a summary of what I did to get past the initial errors based on your instructions:

Updated Makefile to use a different build image so that I could push/pull
- This allowed to build with operator-sdk v0.9.0 executable
Updated CircleCI config to call make force-get-deps
- vendor directory still had operator-sdk v0.9.0 which resulted in compilation error
Added make clean to CircleCI config because the build was complaining about missing file in vendor directory

I got past the initial errors after these changes, and unit tests are passing.

I know that some of my changes, particularly those to the CircleCI config, may be temporary; however, would similar steps have to be performed when other dependences change?

Can we please resume the CI build so that e2e tests are run?

allamand · 2019-08-09T13:26:34Z

@jsanda in my first mind, I was thinking to use of go module with new version of sdk to avoid problem with vendor directory.

Do you think it's better doing this in this PR or in another ?

allamand · 2019-08-09T13:35:06Z

@jsanda there is an error while executing test:

vendor/github.com/operator-framework/operator-sdk/internal/pkg/scaffold/crd.go:94:4: unknown field 'Repo' in struct literal of type "github.com/Orange-OpenSource/cassandra-k8s-operator/vendor/sigs.k8s.io/controller-tools/pkg/crd/generator".Generator

Did you manage execute them locally (using make docker-e2e?

jsanda · 2019-08-09T14:00:42Z

@allamand I am fine with using the go module. Does that mean we introduce support for go modules in this PR? I will need to check to see how to do this. Not sure if it is as simple as something like go build -mod vendor.

With respect to the error, I did not run the make docker-e2e. I will though.

allamand · 2019-08-09T14:14:06Z

not sure of the complexity to introduce go module if it's not too much we can try to include here, othewise will do it in another

jsanda · 2019-08-10T00:11:17Z

@allamand I spent a couple hours trying to get things working with module. I did not have much success. I kept running into different compilation errors.

To be honest, I don't have a strong preference as to whether or not we try to introduce modules as part of this PR or as part of another. I would like to see the move to modules and I would like to see operator-sdk upgraded :) I am happy to help with the migration to modules but may need some help. Maybe we can sync up on slack?

jsanda · 2019-08-10T02:55:04Z

FYI, I opened an issue on the operator-sdk project asking for some help with migrating to modules since I haven't found any docs.

cscetbon · 2019-08-12T15:09:38Z

@jsanda did you try starting it from scratch and see ?

jsanda · 2019-08-12T15:39:51Z

@cscetbon I did go through upgrading the example application in the operator-sdk docs. Initially I ran into the same problem. It turned out that I was missing a step. Running go mod tidy was needed. When I get a chance I need to update and close my operator-sdk ticket.

I still had problems here though even after running go mod tidy. I will diff our go.mod with the one from the example application and hopefully that will make it easier to figure out what dependences need to be fixed.

jsanda · 2019-08-14T20:01:32Z

I just pushed a commit that finally gets operator-sdk generate k8s working with go.mod.

Running operator-sdk build as is done in the build target still needs some work. I will try to address that next.

The Dockerfile generated by v0.9.0 of operator-sdk is a good bit different from what we have. I may need some help with updating it.

jsanda · 2019-08-14T23:57:29Z

I made more progress with my last commit df5d80c. The error in CircleCI looks specific to that environment. I get a different when I run make unit-test locally.

jsanda · 2019-08-15T00:34:22Z

With commit a209dfe I get the same test failure in CircleCI that I get locally.

I also still need to revert the image used in the CircleCI config. I changed it to use on from my GCR repo to get past some errors previously.

jsanda · 2019-08-16T21:08:22Z

@allamand I got unit tests passing. I have had the CirlcCI configured to use the image gcr.io/cassandra-k8s/casskop-build:v0.9.0. I need a orangeopensource/casskop-build:v0.9.0 image to proceed, or run the e2e tests with the current image and see if we have everything passing. Then once all tests pass, create the v0.9.0 image.

cscetbon · 2019-08-16T22:34:10Z

go.mod

+)
+
+replace (
+	k8s.io/api => k8s.io/api v0.0.0-20190222213804-5cb15d344471


why is that one needed when you could just change it in the requires ?

I tried doing that, but the replace directive keeps getting added back. To be honest, I am not sure why as of yet. I have more focused on just getting things working and have not taken much time to investigate.

@jsanda when everything is ready let's try to take a new stab at it ;)

cscetbon · 2019-08-16T22:36:33Z

pkg/controller/cassandracluster/cassandra_status_test.go

@@ -216,9 +226,12 @@ func helperCreateCassandraCluster(t *testing.T, cassandraClusterFileName string)
 	assert.Equal(cc.Status.Phase, api.ClusterPhaseRunning)

 	for _, dcRackName := range dcRackNames {


You don't really need a variable, just delete dcRackNames and use cc.GetDCRackNames() here.

cscetbon · 2019-08-16T22:37:34Z

pkg/controller/cassandracluster/pod.go

-			APIVersion: "v1",
-		},
-	}
+		//Raw: &metav1.ListOptions{


Any reason why we don't need that code anymore ? And if it's not needed, let's remove it definitely

Any reason why we don't need that code anymore ?

It was fixed in kubernetes-sigs/controller-runtime#213. I am removing the commented out code.

allamand · 2019-08-16T22:50:53Z

@jsanda I just trigger tests to see how they doing

jsanda · 2019-08-17T03:14:05Z

I just pushed a couple commits to address comments and to fix the e2e tests. I changed WORKDIR in Makefile to be outside of $GOPATH/src so that modules are automatically enabled. This is consistent with what I have done in other places.

@allamand can you restart tests again please?

jsanda · 2019-08-18T00:45:34Z

@allamand Can you run the e2e tests? I did some testing locally to make sure the docker build worked and one of the e2e tests. I need to test CircleCI now.

cscetbon · 2019-08-18T04:17:56Z

@jsanda I just did

jsanda · 2019-08-18T19:06:01Z

@cscetbon Thanks.

I just pushed another change to set GO111MODULE for docker targets. Can we start e2e tests again please?

cscetbon · 2019-08-19T01:30:17Z

@jsanda /tests

jsanda · 2019-08-19T02:32:48Z

@cscetbon thanks for running tests. Can I kick them off now as well?

@cscetbon and/or @allamand now that all tests pass, we need a orangeopensource/casskop-build:v0.9.0 image and then I change the CircleCI config in my branch back to use the image.

…ompiling

With this commit I can finally get `operator-sdk generate k8s` to run successfully. I also removed dep references from Makefile. Building the docker image still needs some work.

The docker build was failing for me locally with the previous value.

jsanda · 2019-08-22T01:52:04Z

@Orange-cscetbon done!

cscetbon · 2019-08-22T23:23:43Z

pkg/controller/cassandracluster/cassandra_status_test.go

-			t.Fatalf("reconcile: (%v)", err)
+			//We recall Reconcile to update Next rack
+			res, err = rcc.Reconcile(req)
+			if err != nil {


You can do it in one line with if _, err = rcc.Reconcile(req); err != nil {
I also removed res as it's not used

jsanda · 2019-08-23T00:07:49Z

@cscetbon can you start the e2e tests again please?

jsanda · 2019-08-24T01:41:20Z

Thanks!

Now we're just waiting on an orangeopensource/casskop-build:v0.9.0 image. When can we expect that?

allamand · 2019-08-24T09:17:25Z

docker push orangeopensource/casskop-build:v0.9.0

jsanda · 2019-08-26T16:37:50Z

@allamand I have update the CircleCI configuration to use orangeopensource/casskop-build:v0.9.0. Please run the e2e tests when the unit tests finish.

allamand · 2019-08-09T13:15:10Z

Makefile

-BUILD_IMAGE := orangeopensource/casskop-build
+#BUILD_IMAGE ?= orangeopensource/casskop-build
+# This is just for debugging on CircleCI
+BUILD_IMAGE ?= gcr.io/cassandra-k8s/casskop-build



@jsanda Can you change also image in Makefile ?

allamand · 2019-08-27T12:49:54Z

@jsanda can you change in Makefile the WORKDIR

WORKDIR := /go/cassandra-k8s-operator

allamand

Thanks a lot @jsanda :)
I'll add in another PR caching for modules to accelerate the pipeline

* upgrade operator-sdk to v0.9.0 * removed old, commented out code * update OPERATOR_SDK_VERSION var * remove deps target as it no longer exists in v0.9.0 of operator-sdk * Need to run docker-get-deps before docker-build I am not sure if docker-get-deps should be a hard dependency for docker-build but definitely need to update deps right now prior to docker-build since we are upgrading a dependency. * try updated casskop-build image * remove docker-get-deps dependency * update image used by CircleCI * need to force update of vendor directory to update operator-sdk for compiling * previous CircleCI failure complained about missing file in vendir dir. * create a working go.mod file With this commit I can finally get `operator-sdk generate k8s` to run successfully. I also removed dep references from Makefile. Building the docker image still needs some work. * update Dockerfiles and CircleCI config * explicitly enable modules for docker build * move working dir out of $GOPATH/src to activate modules * move more working dirs outside of $GOPATH/src * fix failing tests * change WORKDIR so that go modules are enabled * updates from PR review, and running go mod tidy * try to get workdir values consistent * revert WORKDIR The docker build was failing for me locally with the previous value. * set GO111MODULE=on environment variable for docker targets * found another place modules need to be enabled and some code cleanup * use v0.9.0 orange image * revert BUILD_IMAGE * change WORKDIR

jsanda mentioned this pull request Aug 8, 2019

Add e2e test for cleanup operation #90

Merged

cscetbon reviewed Aug 16, 2019

View reviewed changes

jsanda added 14 commits August 21, 2019 21:50

update image used by CircleCI

c0e5240

need to force update of vendor directory to update operator-sdk for c…

82e06c2

…ompiling

previous CircleCI failure complained about missing file in vendir dir.

acacc26

create a working go.mod file

f3cbe4c

With this commit I can finally get `operator-sdk generate k8s` to run successfully. I also removed dep references from Makefile. Building the docker image still needs some work.

update Dockerfiles and CircleCI config

1d2cf08

explicitly enable modules for docker build

f3b91fd

move working dir out of $GOPATH/src to activate modules

dcf4dfb

move more working dirs outside of $GOPATH/src

b440190

fix failing tests

5fcaa24

change WORKDIR so that go modules are enabled

c6cada1

updates from PR review, and running go mod tidy

fc0d0bb

try to get workdir values consistent

8b95c2b

revert WORKDIR

e37db0c

The docker build was failing for me locally with the previous value.

set GO111MODULE=on environment variable for docker targets

de1550c

jsanda force-pushed the upgrade-operator-sdk branch from 1ed07fb to de1550c Compare August 22, 2019 01:51

cscetbon reviewed Aug 22, 2019

View reviewed changes

found another place modules need to be enabled and some code cleanup

15f2ddb

use v0.9.0 orange image

2372882

allamand reviewed Aug 27, 2019

View reviewed changes

revert BUILD_IMAGE

23700b4

change WORKDIR

81b826d

allamand approved these changes Aug 27, 2019

View reviewed changes

allamand merged commit 32e1952 into Orange-OpenSource:master Aug 27, 2019

		@@ -216,9 +226,12 @@ func helperCreateCassandraCluster(t *testing.T, cassandraClusterFileName string)
		assert.Equal(cc.Status.Phase, api.ClusterPhaseRunning)

		for _, dcRackName := range dcRackNames {

upgrade operator-sdk to v0.9.0 #88

upgrade operator-sdk to v0.9.0 #88

Conversation

jsanda commented Aug 5, 2019

jsanda commented Aug 5, 2019 • edited Loading

allamand commented Aug 5, 2019

jsanda commented Aug 5, 2019

allamand commented Aug 5, 2019 via email

jsanda commented Aug 5, 2019

allamand commented Aug 6, 2019

jsanda commented Aug 6, 2019

allamand commented Aug 9, 2019

allamand commented Aug 9, 2019 • edited Loading

jsanda commented Aug 9, 2019

allamand commented Aug 9, 2019

jsanda commented Aug 10, 2019

jsanda commented Aug 10, 2019

cscetbon commented Aug 12, 2019

jsanda commented Aug 12, 2019

jsanda commented Aug 14, 2019

jsanda commented Aug 14, 2019

jsanda commented Aug 15, 2019

jsanda commented Aug 16, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

allamand commented Aug 16, 2019

jsanda commented Aug 17, 2019

jsanda commented Aug 18, 2019

cscetbon commented Aug 18, 2019

jsanda commented Aug 18, 2019

cscetbon commented Aug 19, 2019

jsanda commented Aug 19, 2019

jsanda commented Aug 22, 2019

Choose a reason for hiding this comment

jsanda commented Aug 23, 2019

jsanda commented Aug 24, 2019

allamand commented Aug 24, 2019 via email • edited Loading

jsanda commented Aug 26, 2019

Choose a reason for hiding this comment

allamand commented Aug 27, 2019

allamand left a comment

Choose a reason for hiding this comment

jsanda commented Aug 5, 2019 •

edited

Loading

allamand commented Aug 9, 2019 •

edited

Loading

jsanda commented Aug 16, 2019 •

edited

Loading

allamand commented Aug 24, 2019 via email •

edited

Loading