-
Notifications
You must be signed in to change notification settings - Fork 68
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
Bump the operator sdk version and fix new / old warnings #463
Conversation
@@ -46,7 +46,7 @@ else | |||
GOBIN=$(shell go env GOBIN) | |||
endif | |||
|
|||
OPERATOR_SDK_VERSION=v1.26.1 | |||
OPERATOR_SDK_VERSION ?= v1.34.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no other line below has ?=, why that changed here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so we can change it from outside if needed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose all the lines below should have that ?= for consistency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
c786380
to
2c9d87c
Compare
Also, fixed all the new complains from operator-sdk. |
Bumping to the latest version and making it possible to override it from outside. Signed-off-by: Federico Paolinelli <fpaoline@redhat.com>
The current version is now fetching also the OPERATOR_SDK env variable which must be fixed. Signed-off-by: Federico Paolinelli <fpaoline@redhat.com>
Making the new operator-sdk happy and removing the deprecation warning. Signed-off-by: Federico Paolinelli <fpaoline@redhat.com>
2c9d87c
to
d1487a7
Compare
Let's remove it completely as metallb doesn't support it yet and the operator sdk was complaining about having it in the bundle but not in the csv. Signed-off-by: Federico Paolinelli <fpaoline@redhat.com>
Adding configuration samples under config/samples so they get reflected in the csv and operatorsdk is happy. Signed-off-by: Federico Paolinelli <fpaoline@redhat.com>
Adding so operatorsdk does not complain. Also, setting to a version old enough that supports our promise to align with the latest supported k8s version. Signed-off-by: Federico Paolinelli <fpaoline@redhat.com>
d1487a7
to
402b388
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
who is using this file and what fails without those changes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
operator-sdk fires a big bad warning
[Deprecation Notice] This version is deprecated and is no longer scaffolded by default since `28 Apr 2021`.The `go/v2` plugin cannot scaffold projects in which CRDs and/or Webhooks have a `v1` API version.Be aware that v1beta1 API for CRDs and Webhooks was deprecated on Kubernetes 1.16 and areremoved as of the Kubernetes 1.22 release. Therefore, since this plugin cannot produce projects thatwork on Kubernetes versions >= 1.22, it is recommended to upgrade your project to the latest versions available.
@@ -9,7 +9,7 @@ resources: | |||
- bases/metallb.io_l2advertisements.yaml | |||
- bases/metallb.io_bgpadvertisements.yaml | |||
- bases/metallb.io_communities.yaml | |||
- bases/metallb.io_servicel2statuses.yaml | |||
# - bases/metallb.io_servicel2statuses.yaml TODO uncomment when metallb support is completed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a plan/issue for that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep, that's being worked out in metallb/metallb#2351
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would personnally add the issue in the comment or remove all the way (but no strong opinion)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about that, but we'll remember as when we'll bump metallb we'll bump the corresponding tests which will fail because the crd is not there.
@@ -54,6 +54,7 @@ spec: | |||
- email: rbryant@redhat.com | |||
name: Russell Bryant | |||
maturity: alpha | |||
minKubeVersion: 1.26.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what fails without that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
operator-sdk fires a warning (it's also in the commit description):
Warning: Value : (metallb-operator.v0.0.0) csv.Spec.minKubeVersion is not informed. It is recommended you provide this information.
@@ -0,0 +1,19 @@ | |||
apiVersion: frrk8s.metallb.io/v1beta1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New samples file were added under the commit bump, with no explaination.
(I am wondering if they were added being are being used somehow or just added for reference)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
they are not under the comimt bump:
It's a specific commit for adding samples
13c5ff3
The commit description explain the reason: those are reflected in the csv samples annotation and operator-sdk doesn't throw a warning
Bumping to the latest version and making it possible to override it from outside.
Also, fixing the various warnings that operator-sdk raises.
Is this a BUG FIX or a FEATURE ?:
What this PR does / why we need it:
Update operator-sdk to a newer version.
Special notes for your reviewer:
Release note: