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

Update operator sdk to 0.14.1 #310

Closed

Conversation

sergioifg94
Copy link
Contributor

Changes

  • Update dependencies in go.mod.
  • Update calls to client.List after change in method signature.
  • Update package imports that changed in newer versions.
  • Clean up tools.go as imports aren't needed with newer version.

Notes

  • Running operator-sdk generate k8s and operator-sdk generate crds fails. Currently looking for a solution.

Update dependencies in `go.mod`.
Update calls to `client.List` after change in method signature.
Update package imports that changed in newer versions.
Clean up `tools.go` as imports aren't needed with newer version.
Copy link

@davidffrench davidffrench left a comment

Choose a reason for hiding this comment

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

Looks like the operator version needs to be changed in the circleCI config as well -

export OPERATOR_SDK_RELEASE_VERSION=v0.8.0

@@ -116,7 +119,21 @@ func main() {
register3scaleVersionInfoMetric()

// Create Service object to expose the metrics port.
_, err = metrics.ExposeMetricsPort(ctx, metricsPort)
servicePorts := []v1.ServicePort{

Choose a reason for hiding this comment

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

@sergioifg94 The migration guide is not good at all explaining the changes needed to this file. Additional changes should be made here.

You should look at the scaffolding from the operator-sdk - https://github.com/operator-framework/operator-sdk/blob/v0.14.1/internal/scaffold/cmd.go#L39-L225

Take a look at my recent PR to the Enmasse operator as well - EnMasseProject/enmasse#3716

@wei-lee
Copy link
Contributor

wei-lee commented Jan 24, 2020

@sergioifg94 I believe some of the errors are related to the templates are not updated. You may need to update the generated templates as well. You can generate the templates by doing the following:

cd <3scale-operator>/pkg/3scale/amp
make clean all

This will cause the new templates to be generated, and you can check them in, and hopefully that will fix the issue.

Change `OPERATOR_SDK_RELEASE_VERSION` environment variable
value to `v0.14.1` in `config.yml`
…rade

Operator SDK introduced a different naming convention for the CRD and CR
YAML files, update the current files to this new convention.
Update generated files by running `operator-sdk generate k8s` and
`operator-sdk generate crds`

BREAKING CHANGE: As newer versions of k8s controller tols don't allow
floats as part of the CRD types, the type of pkg/apis/capabilities/v1alpha1.PlanSpec
fields have to be string. A RegEx validation marker is added to validate
simple decimal numbers (`[0-9]+(\.[0-9]+)?`)
Newer version of k8s.io/apimachinery removed some fields and
types that the tests directly and indirectly relied on:
* The `k8s.io/apimachinery/pkg/apis/meta/v1/GetOptions` and `ListOptions`
  struct removed the `IncludeUnitialized` field, so it's removed from the `wait.go`
  file in the e2e tests
* The `openshift/client-go` module version relied on a type that was removed from
  the `apimachinery` module. Upgrade the dependency to the latest commit
@codeclimate
Copy link

codeclimate bot commented Jan 27, 2020

Code Climate has analyzed commit 151125f and detected 13 issues on this pull request.

Here's the issue category breakdown:

Category Count
Duplication 12
Style 1

View more on Code Climate.

@sergioifg94
Copy link
Contributor Author

Task temporally paused. Some stuff left to do:

  • Check error when running make verify-manifest
  • e2e tests are not passing. Look into what's causing it. A reconciliation error is occurring when the
    tests are run involving the resourceVersion of deployments

@miguelsorianod
Copy link
Contributor

Hi @sergioifg94,

Thank you for the contribution.

I'm starting to look at this PR and what implications does it have for us to update the operator-sdk. After having reviewed the changelog of operator-sdk https://github.com/operator-framework/operator-sdk/blob/master/CHANGELOG.md currently I have the following concerns that I have to analyze to see the viability of updating it:

  • The Kubernetes version that is used now in the code after the upgrade is kubernetes-1.16.2. We are providing support on OpenShift 3.11 which uses Kubernetes 1.11 and on OpenShift 4.x that use more recent versions
    • By looking at the Kubernetes go client https://github.com/kubernetes/client-go#compatibility-matrix it seems some functionalities that might have been added in recent Kubernetes libraries (after the upgrade) might not be available in older Kubernetes versions. If we use the functionalities that are available on both I think it should work, excluding resources that have their APIs in alpha state. An important case of that is the CustomResourceDefinition object, which was in alpha at that time.
    • We have to be very careful with possible changes in the CustomResourceDefinition object, as we have existing users with the CustomResourceDefinition object schemas that were used in Kubernetes 1.11. I think some testing should be needed to make sure it does not break anything when executing the new operator version with customers that have an existing installation, or that we don't have to change anything. Basically do some testing to check if there's some impact in that regard.
  • After the upgrade of operator-sdk the minimum Go version allowed is 1.13. To productize the operator we need to use one of the allowed images to build it. However, the latest available image to productize Go code uses Go 1.12 https://access.redhat.com/containers/?tab=tags#/registry.access.redhat.com/devtools/go-toolset-rhel7. This means we cannot productize it using the allowed image. We need to look if there's a possible alternative to this (maybe just using ubi and manually downloading golang is allowed as productization process?)
  • Apart from 3scale-operator we also have apicast-operator. Using different Go versions in a local environment is inconvenient because we would have to manage different operator-sdk versions, different Go versions, different productization processes (the base image between the operator-sdk has changed from ubi7 to ubi8) so we'd prefer to also upgrade operator-sdk in apicast-operator too in that case. Other inconveniences of having different Go versions is for the local development environment (IDE configurations, having to change between go versions etc...)

Finally, I see the latest released operator-sdk version is currently v0.15.0. Could we update to that latest version in case we decide to go forward with upgrading the operator-sdk version?

Have you tried to generate a new operator-sdk project and compare the files that are common between them to see possible changes between them (in .go code but also on yaml files, directory structure, name of the files etc...)? Just upgrading the operator-sdk version does not incorporate those possible changes. Looking at the changelog I see lots of changes that are not breaking changes (and also several breaking changes) so it's very possible we are not getting some of them if we don't do that.

cc @eguzki so we can discuss this.

Thank you.

@davidffrench
Copy link

davidffrench commented Jan 27, 2020

Thank you very very much for the detailed review on this @miguelsorianod . To give some context, one of the main focuses for upgrading was to include the custom resource operator metrics we get for free from the operator-sdk that was included in v0.9.0. I have some responses inline below to your comment.

Hi @sergioifg94,

Thank you for the contribution.

I'm starting to look at this PR and what implications does it have for us to update the operator-sdk. After having reviewed the changelog of operator-sdk https://github.com/operator-framework/operator-sdk/blob/master/CHANGELOG.md currently I have the following concerns that I have to analyze to see the viability of updating it:

  • The Kubernetes version that is used now in the code after the upgrade is kubernetes-1.16.2. We are providing support on OpenShift 3.11 which uses Kubernetes 1.11 and on OpenShift 4.x that use more recent versions

This is a concern due to a bug in k8s 1.11 with structural schema which the fix for was never backported. The below is a bit lengthy but gives enough context around the issue.

Short Summary
The generated CRDs from the operator-sdk >= 0.11.0 will not work by default on Kubernetes 1.11.

Full Details
The generated CRDs from controller-tools >= 0.2.0 included in the operator-sdk v0.11.0 provide support for structural schema[1] in the generated CRD which is not supported in k8s 1.11 due to a bug that was never backported[2][3]. This adds a type: object to the root of the Spec which then produces an error when applying the CRD to a k8s 1.11 cluster. The workaround is simply removing the type field under spec or removing the status block. However, since the CRDs are auto generated, this should also be some automatic process to avoid human error.

More context - apiextensions.k8s.io/v1beta vs apiextensions.k8s.io/v1
Structural schema is mandatory in apiextensions.k8s.io/v1. apiextensions.k8s.io/v1 was introduced in k8s 1.16 and v1beta1 was deprecated. apiextensions.k8s.io/v1beta1 will no longer be available in k8s 1.19 by default. OpenShift 4.7 will include k8s 1.19 and the current release date is targeting Dec 2 2020. This means even if you decide to use the workaround above, once k8s 1.19 is released, we have no choice but to support two versions of our operators CRDs (e.g. v1alpha1 and v1beta1) if we still need to support k8s < 1.16 at that time.

[1] https://kubernetes.io/docs/tasks/access-kubernetes-api/custom-resources/custom-resource-definitions/#specifying-a-structural-schema
[2] kubernetes/kubernetes#65293
[3] kubernetes/kubernetes#65357

  • By looking at the Kubernetes go client https://github.com/kubernetes/client-go#compatibility-matrix it seems some functionalities that might have been added in recent Kubernetes libraries (after the upgrade) might not be available in older Kubernetes versions. If we use the functionalities that are available on both I think it should work, excluding resources that have their APIs in alpha state. An important case of that is the CustomResourceDefinition object, which was in alpha at that time.

You are correct, the key here is the deprecated & new resources in k8s. Assuming you are still using the same resource versions as before, the newer version of client-go shouldn't cause any issues I am aware of.

  • We have to be very careful with possible changes in the CustomResourceDefinition object, as we have existing users with the CustomResourceDefinition object schemas that were used in Kubernetes 1.11. I think some testing should be needed to make sure it does not break anything when executing the new operator version with customers that have an existing installation, or that we don't have to change anything. Basically do some testing to check if there's some impact in that regard.

You are right, the only issue I have come across was the structural schema problem.

  • After the upgrade of operator-sdk the minimum Go version allowed is 1.13. To productize the operator we need to use one of the allowed images to build it. However, the latest available image to productize Go code uses Go 1.12 https://access.redhat.com/containers/?tab=tags#/registry.access.redhat.com/devtools/go-toolset-rhel7. This means we cannot productize it using the allowed image. We need to look if there's a possible alternative to this (maybe just using ubi and manually downloading golang is allowed as productization process?)

Excellent point. Using ubi and downloading golang seems like a reasonable solution, however, I am not a productisation expert. Would be worth asking whoever the productisation guru is in 3scale.

  • Apart from 3scale-operator we also have apicast-operator. Using different Go versions in a local environment is inconvenient because we would have to manage different operator-sdk versions, different Go versions, different productization processes (the base image between the operator-sdk has changed from ubi7 to ubi8) so we'd prefer to also upgrade operator-sdk in apicast-operator too in that case. Other inconveniences of having different Go versions is for the local development environment (IDE configurations, having to change between go versions etc...)

Makes perfect sense, would be good to have both upgraded at the same time.

Finally, I see the latest released operator-sdk version is currently v0.15.0. Could we update to that latest version in case we decide to go forward with upgrading the operator-sdk version?

Looking at the changes between v0.14 and v0.15, this shouldn't take much effort. Unfortunately, priorities have shifted just today to ensure we can meet our LA deadline for RHMI, so I have unfortunately had to descope this task from the current Sprint.

Have you tried to generate a new operator-sdk project and compare the files that are common between them to see possible changes between them (in .go code but also on yaml files, directory structure, name of the files etc...)? Just upgrading the operator-sdk version does not incorporate those possible changes. Looking at the changelog I see lots of changes that are not breaking changes (and also several breaking changes) so it's very possible we are not getting some of them if we don't do that.

This would be a great exercise to undertake, I agree should be done for this change.

cc @eguzki so we can discuss this.

Thank you.

@eguzki
Copy link
Member

eguzki commented Jan 28, 2020

Regarding supported k8s and OCP version by 3scale operator, as it is specified in doc:

* kubernetes version v1.13.0+
* oc version v4.1+

So, it is not expected to support 3scale operator in OCP 3.11.
@andrewdavidmackenzie can you confirm this?

If the operator is using new version of runtime-controller and other k8s apis, does the operator still support k8s v1.13.0 and OCP 4.1?

Regarding Go version: It is not a stopper for productization. We can build operator in any Go environment. AFAIK there is no such constraint about building operators developed in Go using devtools/go-toolset-rhel7. It should be fine.

@davidffrench
Copy link

If the operator is using new version of runtime-controller and other k8s apis, does the operator still support k8s v1.13.0 and OCP 4.1?

I haven't come across anything that doesn't work. The only thing you need to worry about is deprecated versions for specific k8s resources. But they are very good about not making any breaking changes to v1 resources.

@miguelsorianod
Copy link
Contributor

Hi,

I've been looking at the issue of operator-sdk >= v0.12.0 requiring Go 1.13 (https://github.com/operator-framework/operator-sdk/blob/master/CHANGELOG.md#changed-4) and our productization processes.

For what I've been finding out, no other integration operators are currently productising an operator using Go 1.13.

If we want operator-sdk >= v0.12.0 (the version in this PR) we need Go 1.13 and it seems we currently can't or we are not aware of a way to productize it as there are no productization base images available that we are aware of or any other mechanism that is valid with productization processes.

The possible options I see at this point are:

  • We could upgrade to operator-sdk v0.11.0. For what I understand the changes you needed were available at operator-sdk v0.9.0. Please correct me if I'm wrong with this.
  • Try to make productization happen with Go 1.13 and UBI8 so we can use latest advantages of operator-sdk. For that we need the appropriate productization mechanisms to do so. Do you know how we can make this happen / who we can contact to?

@gsaslis
Copy link
Member

gsaslis commented Feb 11, 2020

Try to make productization happen with Go 1.13 and UBI8 so we can use latest advantages of operator-sdk. For that we need the appropriate productization mechanisms to do so. Do you know how we can make this happen / who we can contact to?

This looks like the way to go for me and, since I imagine all teams will be facing this problem, it makes sense to try to push for a devtools/go-toolset-rhel* base image with Go 1.13 to be released sooner rather than later.

@thomasmaas
Copy link
Member

@sergioifg94 & @davidffrench we will stick with the official red hat dev tools packages meaning that for our next release 2.8, we can use go-toolset-1.12 (but not 1.13 as it hasn't been released yet).

Is there enough value in operator-sdk v0.11.0 for you and will you have time to update this pull request accordingly before the end of this week?

@davidffrench
Copy link

Thank you for the updates @miguelsorianod @gsaslis and @thomasmaas . That makes perfect sense to hold off on the upgrade for this release, I actually thought the 3scale 2.8 code freeze was last Monday. I don't believe anyone will be able to pick this up before the end of the week, I will let you know if anyone can.

@thomasmaas
Copy link
Member

Thanks @davidffrench. Lets get this done for 2.9 (feature freeze for 2.8 was last week, code freeze not yet).

@davidffrench
Copy link

Thanks @thomasmaas . I have synced with the other team leads, we are unable to descope any current task before LA to take on this work. So 2.9 sounds good.

@miguelsorianod
Copy link
Contributor

Hi @sergioifg94 , @wei-lee , @davidffrench ,

We have been working on applying the upgrade of the Go to 1.13.x and operator-sdk version to the latest current one at the time of writing this (v0.15.2) both in this repository and in the apicast-operator one.

The set of applied changes have included the feedback we provided (comparing from an empty project, including non-breaking changes too and other subjects we talked about). We have created a new PR in the case of 3scale-operator to start from a "clean" set of changes and for organization purposes (controlling the granularity of the commits we wanted and changes to them, etc.)

If you are interested in the changes applied, the PRs related to the changes are:
#331
3scale/apicast-operator#54

The changes have already been merged into master for both repositories, but take into account that they are expected to be available on the 3scale 2.9 release (unless some unexpected problem is detected in the future related to the upgrade), as for 2.8 there's code freeze since some weeks ago and it's on the testing/productization process already.

If you want you can start trying the new changes in the master branch, taking and into account that changes happen to that branch and is not tied to any specific release, is where development progress happens.

I'm closing this PR as it's been already implemented.

@davidffrench
Copy link

Thank you @miguelsorianod . I really appreciate the status update and am happy we could help in some small way. I took a look at #331 and it looks great.

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.

7 participants