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.

Sign up for GitHub

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

Jump to bottom

[Breaking change] Upgrade quarkus from 1.2.1.Final to 1.9.0.Final #46

Merged
merged 8 commits into from Dec 10, 2020
Merged

[Breaking change] Upgrade quarkus from 1.2.1.Final to 1.9.0.Final #46

merged 8 commits into from Dec 10, 2020

Conversation

dlbock
Copy link
Contributor

@dlbock dlbock commented Dec 4, 2020 •
edited

Why

The version of the quarkus dependency is sorely outdated (1.2.1.Final). In order for us to leverage the v1 api of CustomResourceDefinition (which we will need to do soon since v1beta1/CustomResourceDefinition is deprecated and will be removed in Kubernetes v1.22+), we need at least kubernetes-client 4.11.x. Version 1.9.0.Final of quarkus gives us kubernetes-client 4.12. The last version update of kubernetes-client Quarkus did was to 4.10.3 before that.

Note: This is a breaking change as the version of kubernetes-client that's used when calling .createOrReplace on the Custom Resource's status requires an additional create permission in the ClusterRole for instana.io.

What

  • The main change here is the version bump of quarkus from 1.2.1.Final to 1.9.0.Final and all the necessary code changes for it to compile.
  • The other change to point out is that we had to add the create permissions to the instana.io apiGroup in the Operator's ClusterRole in instana-agent-operator.yaml. This is due to a change in [kubernetes-client](https://github.com/fabric8io/kubernetes-client/issues/2292) when calling createOrReplace, it will first try a create and if it fails with a conflict exception, then it'll try an update, which means that the operator will need create permissions in the instana.io apiGroup
  • Added some additional error messaging to inform the user that they should check the operator has updated cluster role permissions if they encounter an error when applying the custom resource
  • Also updated the operator-artifacts.jsonnet template to include the version of the instana/instana-agent-operator image tag in the generated instana-agent-operator.yaml to help ensure that the operators using a previous tag would still work if we introduce a breaking change in a new operator image that requires an updated instana-agent-operator.yaml (like the one we're doing now)
  • No longer pushing the latest tag for the operator image in order to tie the operator yaml updates more closely with the operator image updates and avoid the situation where the customers are pulling the latest operator image containing a breaking change but not explicitly updating the operator yaml

The auxiliary changes are for easing the end-to-end testing with Kind with the introduction of 2 scripts that will:

  1. Create a local Kind cluster with 1 control plane and 2 nodes (with support for macOS/Darwin and Linux), build the operator Docker image with the new maven artifacts and load that Docker image into the Kind cluster, pull the latest agent Docker image and load that into the Kind cluster
  2. Delete that local Kind cluster

@dlbock dlbock self-assigned this Dec 4, 2020
@dlbock
Upgrade quarkus from 1.2.1.Final to the latest 1.9.0.Final in order t…
d07f72b 
…o include kubernetes-client of at least 4.11.x (4.12.x is included in 1.9.0.Final).

This is necessary for us to be able to leverage the v1 API for CustomResourceDefinition as v1beta1 is deprecated and will soon be removed.
@dlbock dlbock requested a review from wiggzz December 4, 2020 21:18
@dlbock dlbock requested a review from wiggzz December 7, 2020 23:14
@dlbock dlbock force-pushed the upgrade-quarkus branch 3 times, most recently from 796ae4e to 5391a5e Compare December 7, 2020 23:38
@dlbock
Add additional error messaging if we fail to update the CustomResourc…
693533e 
…e status due to a lack of permissions
@dlbock
Update the local testing with Kind with some scripts to create and de…
e406181 
…lete the cluster
@dlbock
Update the operator-artifacts.jsonnet template to include the version…
9de49c2 
… of the instana/instana-agent-operator image tag in the generated instana-agent-operator.yaml

This will help ensure that the latest operator image version does not get pulled if it requires the customer to also update the instana-agent-operator.yaml with the latest changes.
wiggzz
wiggzz reviewed Dec 9, 2020
View reviewed changes
Copy link
Collaborator

@wiggzz wiggzz left a comment

Choose a reason for hiding this comment

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

Nice. Looks good.

olm/operator-resources/operator-artifacts.jsonnet
},
spec+:
super.spec + {
containers: std.mapWithIndex(function(i, c) if i == 0 then c + {image: c.image + ":" + version} else c, super.containers)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this the full version 0.3.9? This is good for now - but ideally in the future we should probably create major version tags, (so, in this case 0) and then use the major version tag here as opposed to the full version. But honestly we probably need a proper update strategy other than just "wait for the operator to crash and re-pull"... so, this is good.

Copy link
Contributor Author

@dlbock dlbock Dec 9, 2020 •
edited

Choose a reason for hiding this comment

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

Yeah this is the full version, so it'll look like this: image: instana/instana-agent-operator:0.3.9

@dlbock
Do not push the latest tag for the operator Docker image anymore as w…
d5cd161 
…e'll need to tie the operator image version to the operator yaml version to ensure customers update the operator yaml in order to get the latest image
@dlbock
Update Instana docs link from docs.instana.io to www.instana.com/docs
610c77b
dlbock
dlbock commented Dec 10, 2020
View reviewed changes
Jenkinsfile
image.push()
image.push('latest')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@wiggzz After discussion with Henning/Miel/Et al., we decided to no longer push the latest tag and to make the dependency between the operator image and the operator yaml more explicit, i.e. if a customer wants a more recent version of the operator, they have to update their operator yaml.

@dlbock
No need to add the version for the instana/instana-agent-operator ima…
581a616 
…ge within the Deployment when generating the ClusterServiceVersion yaml as we have already placed the version there (and no longer using the latest tag)
@dlbock
Change the format of the error message that is logged when we fail to…
f5ac061 
… update the CustomResource status due to a lack of permissions
@dlbock dlbock changed the title Upgrade quarkus from 1.2.1.Final to 1.9.0.Final [Breaking change] Upgrade quarkus from 1.2.1.Final to 1.9.0.Final Dec 10, 2020
@dlbock
Copy link
Contributor Author

dlbock commented Dec 10, 2020

Tested the image change with and without the yaml change. Also ensured that the OLM and RedHat artifacts generated only contain the expected changes.

@dlbock dlbock merged commit 19c2430 into master Dec 10, 2020
@dlbock dlbock deleted the upgrade-quarkus branch December 10, 2020 20:24
@dlbock
Copy link
Contributor Author

dlbock commented Dec 10, 2020

Will release v1.0.0 closer to Release 192

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wiggzz wiggzz wiggzz approved these changes

Assignees

@dlbock dlbock

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@dlbock @wiggzz