-
Notifications
You must be signed in to change notification settings - Fork 176
Mc/rox 33465 openshift autosensing 6 #19968
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
Changes from all commits
9169241
c6c020a
9f57afc
de59b9d
e342a1d
9ae2bd1
e6632dd
a23285a
33cf606
430dd3f
4184e4d
2639886
3691373
772f586
4039256
150a1eb
1e0512a
5dc132c
653812f
e662b82
5954132
c278bdf
71c3c3e
1facce1
a8ca7fd
8dbc34d
ee842ce
ea80cab
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,13 +4,11 @@ | |
| This function detects the OpenShift version automatically based on the cluster the Helm chart is installed onto. | ||
| It writes the result to ._rox.env.openshift as an integer. | ||
| Possible results are: | ||
| - 3 (OpenShift 3) | ||
| - 4 (OpenShift 4) | ||
| - 0 (Non-Openshift cluster) | ||
|
|
||
| If "true" is passed for $._rox.env.openshift the OpenShift version is detected based on the Kubernetes cluster version. | ||
| If the Kubernetes version is not available (i.e. when using Helm template) auto-sensing falls back on OpenShift 3 to be | ||
| backward compatible. | ||
| If "true" is passed for $._rox.env.openshift, this is unconditionally mapped to OpenShift version "4", because that is the only | ||
| major version we currently support. | ||
|
Comment on lines
+10
to
+11
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue (bug_risk): The documented behavior for passing "true" does not match the implemented behavior. This comment now states that passing "true" for |
||
| */}} | ||
|
|
||
| {{ define "srox.autoSenseOpenshiftVersion" }} | ||
|
|
@@ -20,31 +18,25 @@ | |
|
|
||
| {{/* Infer OpenShift, if needed */}} | ||
| {{ if kindIs "invalid" $env.openshift }} | ||
| {{/* The API GroupVersion project.openshift.io/v1 contains the core OpenShift API 'Project' of | ||
| compatibility level 1, which comes with the strongest stability guarantees among the OpenShift APIs. | ||
| This API is available in OpenShift 3.x and 4.x. */}} | ||
| {{ $_ := set $env "openshift" (has "project.openshift.io/v1" $._rox._apiServer.apiResources) }} | ||
| {{/* This CRD API reliably indicates OpenShift 4. */}} | ||
| {{ $_ := set $env "openshift" (has "config.openshift.io/v1" $._rox._apiServer.apiResources) }} | ||
| {{- if $env.openshift -}} | ||
| {{- include "srox.note" (list $ (printf "Based on API server properties, we have inferred that you are deploying into an OpenShift 4.x cluster.")) -}} | ||
| {{- end -}} | ||
| {{ end }} | ||
|
|
||
| {{/* Infer openshift version */}} | ||
| {{ if and $env.openshift (kindIs "bool" $env.openshift) }} | ||
| {{/* Parse and add KubeVersion as semver from built-in resources. This is necessary to compare valid integer numbers. */}} | ||
| {{ $kubeVersion := semver $.Capabilities.KubeVersion.Version }} | ||
|
|
||
| {{/* Default to OpenShift 3 if no openshift resources are available, i.e. in helm template commands */}} | ||
| {{ if not (has "project.openshift.io/v1" $._rox._apiServer.apiResources) }} | ||
| {{ $_ := set $._rox.env "openshift" 3 }} | ||
| {{ else if gt $kubeVersion.Minor 11 }} | ||
| {{ $_ := set $env "openshift" 4 }} | ||
| {{ else }} | ||
| {{ $_ := set $env "openshift" 3 }} | ||
| {{ end }} | ||
| {{ include "srox.note" (list $ (printf "Based on API server properties, we have inferred that you are deploying into an OpenShift %d.x cluster. Set the `env.openshift` property explicitly to 3 or 4 to override the auto-sensed value." $env.openshift)) }} | ||
| {{/* We only support OpenShift 4. */}} | ||
| {{ $_ := set $env "openshift" 4 }} | ||
| {{ end }} | ||
|
|
||
| {{ if not (kindIs "bool" $env.openshift) }} | ||
| {{ $_ := set $env "openshift" (int $env.openshift) }} | ||
| {{ else if not $env.openshift }} | ||
| {{ $_ := set $env "openshift" 0 }} | ||
| {{ end }} | ||
|
|
||
| {{- if and (ne $env.openshift 0) (ne $env.openshift 4) -}} | ||
| {{- include "srox.fail" (printf "You have specified OpenShift version %d.x, but only version 4.x is currently supported." $env.openshift) -}} | ||
| {{- end -}} | ||
|
|
||
| {{ end }} | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -7,7 +7,7 @@ clusterConfig: | |||||||||||||||||||||||||||
| {{- if not ._rox.env.openshift }} | ||||||||||||||||||||||||||||
| type: KUBERNETES_CLUSTER | ||||||||||||||||||||||||||||
| {{- else }} | ||||||||||||||||||||||||||||
| type: {{ if eq (int ._rox.env.openshift) 4 -}} OPENSHIFT4_CLUSTER {{- else -}} OPENSHIFT_CLUSTER {{ end }} | ||||||||||||||||||||||||||||
| type: OPENSHIFT4_CLUSTER | ||||||||||||||||||||||||||||
| {{- end }} | ||||||||||||||||||||||||||||
|
Comment on lines
7
to
11
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do not silently coerce deprecated OpenShift 3 input to OpenShift 4. At Line 10, any truthy Proposed fix {{- if not ._rox.env.openshift }}
type: KUBERNETES_CLUSTER
- {{- else }}
+ {{- else if eq (._rox.env.openshift | int) 3 }}
+ {{- fail "OpenShift 3.x is not supported anymore" }}
+ {{- else }}
type: OPENSHIFT4_CLUSTER
{{- end }}Please also add one helmtest case asserting As per coding guidelines, "Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity." 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||
| mainImage: {{ coalesce ._rox.image.main._abbrevImageRef ._rox.image.main.fullRef }} | ||||||||||||||||||||||||||||
| collectorImage: {{ coalesce ._rox.image.collector._abbrevImageRef ._rox.image.collector.fullRef }} | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
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.
Telemetry still reports the deprecated OpenShift 3 platform label.
At Line 119, all OpenShift deployments are still tagged as
OPENSHIFT_CLUSTER, which now represents deprecated/unsupported OpenShift 3 semantics. This will misclassify OpenShift 4 telemetry.Suggested fix
if env.Openshift.BooleanSetting() { - // Should probably be changed to OPENSHIFT4_CLUSTER, but since this is production code, keeping it for now - // due to compatibility concerns. - orchestrator = storage.ClusterType_OPENSHIFT_CLUSTER.String() + orchestrator = storage.ClusterType_OPENSHIFT4_CLUSTER.String() }📝 Committable suggestion
🤖 Prompt for AI Agents