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
[4.3] Bug 1782574: Restrict minimum kube version #73
[4.3] Bug 1782574: Restrict minimum kube version #73
Conversation
@gnufied: This pull request references Bugzilla bug 1782574, which is invalid:
Comment In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
@@ -49,6 +49,7 @@ spec: | |||
- name: Source Repository | |||
url: https://github.com/openshift/local-storage-operator | |||
version: 4.3.0 | |||
minKubeVersion: 1.14.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.
I'm wondering if we can use 1.16 for 4.3 and 1.17 for 4.4 in order to prevent people from using the wrong manifests.
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.
As I understand it - minKubeVersion
is only designed to have a last minute switch to stop an operator from being installed on a incompatible kube release. I for example when tested this - this indeed prevented operator from being installed but there were no hints in the UI that operator was prevented from being installed because of kube version mis-match.
In a nutshell - this is not a good mechanism to specify version ranges of kubernetes(or openshift). The manifests are still being displayed to the user because they come from same operatorsource. I would rather OLM team provided a better mechanism for this. May be index images will solve it.
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.
Should be 4.2 requirement noted somewhere in the operator description then?
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.
yeah I can update description of 4.2 operator.
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.
OLM is working on a better mechanism how to restrict operator version to OCP version, https://coreos.slack.com/archives/C3VS0LV41/p1576081661098700?thread_ts=1576079878.090000&cid=C3VS0LV41
I like it as it is in this PR, let's use minKubeVersion
to prevent users installing it on 4.1, but then it's up to the user to subscribe to the right channel. Users can install newest 4.3 operator in 4.2, IMO it should work. We must be careful when triaging customer issues, so the operator version is in sync with OCP version and suggest upgrade / downgrade otherwise.
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bertinatto, gnufied The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/hold Adding hold in case #73 (comment) should be addressed. |
out of curiosity... |
@jsafrane: This pull request references Bugzilla bug 1782574, which is invalid:
Comment In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
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.
Please sync the description about OCP 4.2 as minimal version.
@@ -48,6 +48,7 @@ spec: | |||
- name: Source Repository | |||
url: https://github.com/openshift/local-storage-operator | |||
version: 4.2.0 | |||
minKubeVersion: 1.14.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.
in master you removed whole manifests/4.2/local-storage-operator.v4.2.0.clusterserviceversion.yaml
. Should we keep it in 4.3?
/bugzilla refresh |
@jsafrane: This pull request references Bugzilla bug 1782574, which is invalid:
Comment In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/bugzilla refresh The requirements for Bugzilla bugs have changed, recalculating validity. |
@openshift-bot: This pull request references Bugzilla bug 1782574, which is invalid:
Comment In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/bugzilla refresh |
@gnufied: This pull request references Bugzilla bug 1782574, which is invalid:
Comment In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/hold cancel |
/bugzilla refresh |
@bertinatto: This pull request references Bugzilla bug 1782574, which is invalid:
Comment In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/bugzilla refresh |
@gnufied: This pull request references Bugzilla bug 1782574, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
@gnufied: All pull requests linked via external trackers have merged. Bugzilla bug 1782574 has been moved to the MODIFIED state. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1782574