Skip to content

Commit 85c8259

Browse files
mprahlopenshift-merge-bot[bot]
mprahl
authored and
openshift-merge-bot[bot]
committed
Check the hosting cluster vendor when in hosted mode
When the hosting cluster is not OpenShift, OpenShift configuration should not be enabled on the hosting cluster, regardless of if the hosted cluster is OpenShift. Relates: https://issues.redhat.com/browse/ACM-13204 Signed-off-by: mprahl <[email protected]>
1 parent db6e644 commit 85c8259

23 files changed

+350
-265
lines changed

‎pkg/addon/common.go‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,24 @@ func NewRegistrationOption(
135135
}
136136
}
137137

138+
func GetClusterVendor(cluster *clusterv1.ManagedCluster) string {
139+
var vendor string
140+
// Don't just set it to the value in the label, it might be something like "auto-detect"
141+
if cluster.Labels["vendor"] == "OpenShift" {
142+
vendor = "OpenShift"
143+
}
144+
145+
for _, cc := range cluster.Status.ClusterClaims {
146+
if cc.Name == "product.open-cluster-management.io" {
147+
vendor = cc.Value
148+
149+
break
150+
}
151+
}
152+
153+
return vendor
154+
}
155+
138156
func GetAndAddAgent(
139157
ctx context.Context,
140158
mgr addonmanager.AddonManager,

‎pkg/addon/configpolicy/agent_addon.go‎

Lines changed: 125 additions & 115 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,14 @@ import (
88
"strconv"
99

1010
"github.com/openshift/library-go/pkg/controller/controllercmd"
11+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1112
"open-cluster-management.io/addon-framework/pkg/addonfactory"
1213
"open-cluster-management.io/addon-framework/pkg/addonmanager"
1314
"open-cluster-management.io/addon-framework/pkg/agent"
1415
"open-cluster-management.io/addon-framework/pkg/utils"
1516
addonapiv1alpha1 "open-cluster-management.io/api/addon/v1alpha1"
1617
addonv1alpha1client "open-cluster-management.io/api/client/addon/clientset/versioned"
18+
clusterv1client "open-cluster-management.io/api/client/cluster/clientset/versioned"
1719
clusterv1 "open-cluster-management.io/api/cluster/v1"
1820
ctrl "sigs.k8s.io/controller-runtime"
1921

@@ -39,11 +41,12 @@ type UserArgs struct {
3941
}
4042

4143
type UserValues struct {
42-
GlobalValues policyaddon.GlobalValues `json:"global,"`
43-
KubernetesDistribution string `json:"kubernetesDistribution"`
44-
Prometheus map[string]interface{} `json:"prometheus"`
45-
OperatorPolicy map[string]interface{} `json:"operatorPolicy"`
46-
UserArgs UserArgs `json:"args,"`
44+
GlobalValues policyaddon.GlobalValues `json:"global,"`
45+
KubernetesDistribution string `json:"kubernetesDistribution"`
46+
HostingKubernetesDistribution string `json:"hostingKubernetesDistribution"`
47+
Prometheus map[string]interface{} `json:"prometheus"`
48+
OperatorPolicy map[string]interface{} `json:"operatorPolicy"`
49+
UserArgs UserArgs `json:"args,"`
4750
}
4851

4952
// FS go:embed
@@ -60,138 +63,145 @@ var agentPermissionFiles = []string{
6063
"manifests/hubpermissions/rolebinding.yaml",
6164
}
6265

63-
func getValues(cluster *clusterv1.ManagedCluster,
64-
addon *addonapiv1alpha1.ManagedClusterAddOn,
66+
func getValues(ctx context.Context, clusterClient *clusterv1client.Clientset) func(*clusterv1.ManagedCluster,
67+
*addonapiv1alpha1.ManagedClusterAddOn,
6568
) (addonfactory.Values, error) {
66-
userValues := UserValues{
67-
GlobalValues: policyaddon.GlobalValues{
68-
ImagePullPolicy: "IfNotPresent",
69-
ImagePullSecret: "open-cluster-management-image-pull-credentials",
70-
ImageOverrides: map[string]string{
71-
"config_policy_controller": os.Getenv("CONFIG_POLICY_CONTROLLER_IMAGE"),
69+
return func(
70+
cluster *clusterv1.ManagedCluster, addon *addonapiv1alpha1.ManagedClusterAddOn,
71+
) (addonfactory.Values, error) {
72+
userValues := UserValues{
73+
GlobalValues: policyaddon.GlobalValues{
74+
ImagePullPolicy: "IfNotPresent",
75+
ImagePullSecret: "open-cluster-management-image-pull-credentials",
76+
ImageOverrides: map[string]string{
77+
"config_policy_controller": os.Getenv("CONFIG_POLICY_CONTROLLER_IMAGE"),
78+
},
79+
ProxyConfig: map[string]string{
80+
"HTTP_PROXY": "",
81+
"HTTPS_PROXY": "",
82+
"NO_PROXY": "",
83+
},
7284
},
73-
ProxyConfig: map[string]string{
74-
"HTTP_PROXY": "",
75-
"HTTPS_PROXY": "",
76-
"NO_PROXY": "",
85+
Prometheus: map[string]interface{}{},
86+
OperatorPolicy: map[string]interface{}{},
87+
UserArgs: UserArgs{
88+
UserArgs: policyaddon.UserArgs{
89+
LogEncoder: "console",
90+
LogLevel: 0,
91+
PkgLogLevel: -1,
92+
},
93+
// Defaults from `values.yaml` will be used if these stay at 0.
94+
EvaluationConcurrency: 0,
95+
ClientQPS: 0, // will be set based on concurrency if not explicitly set
96+
ClientBurst: 0, // will be set based on concurrency if not explicitly set
7797
},
78-
},
79-
Prometheus: map[string]interface{}{},
80-
OperatorPolicy: map[string]interface{}{},
81-
UserArgs: UserArgs{
82-
UserArgs: policyaddon.UserArgs{
83-
LogEncoder: "console",
84-
LogLevel: 0,
85-
PkgLogLevel: -1,
86-
},
87-
// Defaults from `values.yaml` will be used if these stay at 0.
88-
EvaluationConcurrency: 0,
89-
ClientQPS: 0, // will be set based on concurrency if not explicitly set
90-
ClientBurst: 0, // will be set based on concurrency if not explicitly set
91-
},
92-
}
98+
}
9399

94-
// Don't just set it to the value in the label, it might be something like "auto-detect"
95-
if cluster.Labels["vendor"] == "OpenShift" {
96-
userValues.KubernetesDistribution = "OpenShift"
97-
}
100+
userValues.KubernetesDistribution = policyaddon.GetClusterVendor(cluster)
98101

99-
for _, cc := range cluster.Status.ClusterClaims {
100-
if cc.Name == "product.open-cluster-management.io" {
101-
userValues.KubernetesDistribution = cc.Value
102+
hostingClusterName := addon.Annotations["addon.open-cluster-management.io/hosting-cluster-name"]
103+
if hostingClusterName != "" {
104+
hostingCluster, err := clusterClient.ClusterV1().ManagedClusters().Get(
105+
ctx, hostingClusterName, metav1.GetOptions{},
106+
)
107+
if err != nil {
108+
return nil, err
109+
}
102110

103-
break
111+
userValues.HostingKubernetesDistribution = policyaddon.GetClusterVendor(hostingCluster)
112+
} else {
113+
userValues.HostingKubernetesDistribution = userValues.KubernetesDistribution
104114
}
105-
}
106115

107-
// Enable Prometheus metrics by default on OpenShift
108-
userValues.Prometheus["enabled"] = userValues.KubernetesDistribution == "OpenShift"
116+
// Enable Prometheus metrics by default on OpenShift
117+
userValues.Prometheus["enabled"] = userValues.HostingKubernetesDistribution == "OpenShift"
109118

110-
// Disable OperatorPolicy if the cluster is not on OpenShift version 4.y
111-
userValues.OperatorPolicy["disabled"] = cluster.Labels["openshiftVersion-major"] != "4"
119+
// Disable OperatorPolicy if the cluster is not on OpenShift version 4.y
120+
userValues.OperatorPolicy["disabled"] = cluster.Labels["openshiftVersion-major"] != "4"
112121

113-
// Set the default namespace for OperatorPolicy for OpenShift 4
114-
if cluster.Labels["openshiftVersion-major"] == "4" {
115-
userValues.OperatorPolicy["defaultNamespace"] = "openshift-operators"
116-
}
122+
// Set the default namespace for OperatorPolicy for OpenShift 4
123+
if cluster.Labels["openshiftVersion-major"] == "4" {
124+
userValues.OperatorPolicy["defaultNamespace"] = "openshift-operators"
125+
}
117126

118-
annotations := addon.GetAnnotations()
127+
annotations := addon.GetAnnotations()
119128

120-
if val, ok := annotations[policyaddon.PolicyLogLevelAnnotation]; ok {
121-
logLevel := policyaddon.GetLogLevel(addonName, val)
122-
userValues.UserArgs.UserArgs.LogLevel = logLevel
123-
userValues.UserArgs.UserArgs.PkgLogLevel = logLevel - 2
124-
}
129+
if val, ok := annotations[policyaddon.PolicyLogLevelAnnotation]; ok {
130+
logLevel := policyaddon.GetLogLevel(addonName, val)
131+
userValues.UserArgs.UserArgs.LogLevel = logLevel
132+
userValues.UserArgs.UserArgs.PkgLogLevel = logLevel - 2
133+
}
125134

126-
if val, ok := annotations[evaluationConcurrencyAnnotation]; ok {
127-
value, err := strconv.ParseUint(val, 10, 8)
128-
if err != nil {
129-
log.Error(err, fmt.Sprintf(
130-
"Failed to verify '%s' annotation value '%s' for component %s (falling back to default value %d)",
131-
evaluationConcurrencyAnnotation, val, addonName, userValues.UserArgs.EvaluationConcurrency),
132-
)
133-
} else {
134-
// This is safe because we specified the uint8 in ParseUint
135-
userValues.UserArgs.EvaluationConcurrency = uint8(value)
135+
if val, ok := annotations[evaluationConcurrencyAnnotation]; ok {
136+
value, err := strconv.ParseUint(val, 10, 8)
137+
if err != nil {
138+
log.Error(err, fmt.Sprintf(
139+
"Failed to verify '%s' annotation value '%s' for component %s (falling back to default value %d)",
140+
evaluationConcurrencyAnnotation, val, addonName, userValues.UserArgs.EvaluationConcurrency),
141+
)
142+
} else {
143+
// This is safe because we specified the uint8 in ParseUint
144+
userValues.UserArgs.EvaluationConcurrency = uint8(value)
145+
}
136146
}
137-
}
138147

139-
if val, ok := annotations[clientQPSAnnotation]; ok {
140-
value, err := strconv.ParseUint(val, 10, 8)
141-
if err != nil {
142-
log.Error(err, fmt.Sprintf(
143-
"Failed to verify '%s' annotation value '%s' for component %s (falling back to default value %d)",
144-
clientQPSAnnotation, val, addonName, userValues.UserArgs.ClientQPS),
145-
)
146-
} else {
147-
// This is safe because we specified the uint8 in ParseUint
148-
userValues.UserArgs.ClientQPS = uint8(value)
148+
if val, ok := annotations[clientQPSAnnotation]; ok {
149+
value, err := strconv.ParseUint(val, 10, 8)
150+
if err != nil {
151+
log.Error(err, fmt.Sprintf(
152+
"Failed to verify '%s' annotation value '%s' for component %s (falling back to default value %d)",
153+
clientQPSAnnotation, val, addonName, userValues.UserArgs.ClientQPS),
154+
)
155+
} else {
156+
// This is safe because we specified the uint8 in ParseUint
157+
userValues.UserArgs.ClientQPS = uint8(value)
158+
}
159+
} else { // not set explicitly
160+
userValues.UserArgs.ClientQPS = userValues.UserArgs.EvaluationConcurrency * 15
149161
}
150-
} else { // not set explicitly
151-
userValues.UserArgs.ClientQPS = userValues.UserArgs.EvaluationConcurrency * 15
152-
}
153162

154-
if val, ok := annotations[clientBurstAnnotation]; ok {
155-
value, err := strconv.ParseUint(val, 10, 8)
156-
if err != nil {
157-
log.Error(err, fmt.Sprintf(
158-
"Failed to verify '%s' annotation value '%s' for component %s (falling back to default value %d)",
159-
clientBurstAnnotation, val, addonName, userValues.UserArgs.ClientBurst),
160-
)
161-
} else {
162-
// This is safe because we specified the uint8 in ParseUint
163-
userValues.UserArgs.ClientBurst = uint8(value)
163+
if val, ok := annotations[clientBurstAnnotation]; ok {
164+
value, err := strconv.ParseUint(val, 10, 8)
165+
if err != nil {
166+
log.Error(err, fmt.Sprintf(
167+
"Failed to verify '%s' annotation value '%s' for component %s (falling back to default value %d)",
168+
clientBurstAnnotation, val, addonName, userValues.UserArgs.ClientBurst),
169+
)
170+
} else {
171+
// This is safe because we specified the uint8 in ParseUint
172+
userValues.UserArgs.ClientBurst = uint8(value)
173+
}
174+
} else if userValues.UserArgs.EvaluationConcurrency != 0 {
175+
// only scale with concurrency if concurrency was set.
176+
userValues.UserArgs.ClientBurst = userValues.UserArgs.EvaluationConcurrency*22 + 1
164177
}
165-
} else if userValues.UserArgs.EvaluationConcurrency != 0 {
166-
// only scale with concurrency if concurrency was set.
167-
userValues.UserArgs.ClientBurst = userValues.UserArgs.EvaluationConcurrency*22 + 1
168-
}
169178

170-
if val, ok := annotations[prometheusEnabledAnnotation]; ok {
171-
valBool, err := strconv.ParseBool(val)
172-
if err != nil {
173-
log.Error(err, fmt.Sprintf(
174-
"Failed to verify '%s' annotation value '%s' for component %s (falling back to default value %v)",
175-
prometheusEnabledAnnotation, val, addonName, userValues.Prometheus["enabled"]),
176-
)
177-
} else {
178-
userValues.Prometheus["enabled"] = valBool
179+
if val, ok := annotations[prometheusEnabledAnnotation]; ok {
180+
valBool, err := strconv.ParseBool(val)
181+
if err != nil {
182+
log.Error(err, fmt.Sprintf(
183+
"Failed to verify '%s' annotation value '%s' for component %s (falling back to default value %v)",
184+
prometheusEnabledAnnotation, val, addonName, userValues.Prometheus["enabled"]),
185+
)
186+
} else {
187+
userValues.Prometheus["enabled"] = valBool
188+
}
179189
}
180-
}
181190

182-
if val, ok := annotations[operatorPolicyDisabledAnnotation]; ok {
183-
valBool, err := strconv.ParseBool(val)
184-
if err != nil {
185-
log.Error(err, fmt.Sprintf(
186-
"Failed to verify '%s' annotation value '%s' for component %s (falling back to default value %v)",
187-
operatorPolicyDisabledAnnotation, val, addonName, userValues.OperatorPolicy["disabled"]),
188-
)
189-
} else {
190-
userValues.OperatorPolicy["disabled"] = valBool
191+
if val, ok := annotations[operatorPolicyDisabledAnnotation]; ok {
192+
valBool, err := strconv.ParseBool(val)
193+
if err != nil {
194+
log.Error(err, fmt.Sprintf(
195+
"Failed to verify '%s' annotation value '%s' for component %s (falling back to default value %v)",
196+
operatorPolicyDisabledAnnotation, val, addonName, userValues.OperatorPolicy["disabled"]),
197+
)
198+
} else {
199+
userValues.OperatorPolicy["disabled"] = valBool
200+
}
191201
}
192-
}
193202

194-
return addonfactory.JsonStructToValues(userValues)
203+
return addonfactory.JsonStructToValues(userValues)
204+
}
195205
}
196206

197207
// mandateValues sets deployment variables regardless of user overrides. As a result, caution should
@@ -239,7 +249,7 @@ func GetAgentAddon(ctx context.Context, controllerContext *controllercmd.Control
239249
addonfactory.ToAddOnNodePlacementValues,
240250
addonfactory.ToAddOnCustomizedVariableValues,
241251
),
242-
getValues,
252+
getValues(ctx, clusterClient),
243253
addonfactory.GetValuesFromAddonAnnotation,
244254
mandateValues,
245255
).

‎pkg/addon/configpolicy/manifests/managedclusterchart/templates/auth_cluster_role.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
# Note that this only needs to be created in hosted mode since the controller has all permissions on the managed
33
# cluster.
44

5-
{{- if and (eq .Values.installMode "Hosted") .Values.prometheus.enabled (eq .Values.kubernetesDistribution "OpenShift") }}
5+
{{- if and (eq .Values.installMode "Hosted") .Values.prometheus.enabled (eq .Values.hostingKubernetesDistribution "OpenShift") }}
66
apiVersion: rbac.authorization.k8s.io/v1
77
kind: ClusterRole
88
metadata:

‎pkg/addon/configpolicy/manifests/managedclusterchart/templates/auth_cluster_role_binding.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
# Note that this only needs to be created in hosted mode since the controller has all permissions on the managed
33
# cluster.
44

5-
{{- if and (eq .Values.installMode "Hosted") .Values.prometheus.enabled (eq .Values.kubernetesDistribution "OpenShift") }}
5+
{{- if and (eq .Values.installMode "Hosted") .Values.prometheus.enabled (eq .Values.hostingKubernetesDistribution "OpenShift") }}
66
apiVersion: rbac.authorization.k8s.io/v1
77
kind: ClusterRoleBinding
88
metadata:

‎pkg/addon/configpolicy/manifests/managedclusterchart/templates/deployment.yaml‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ spec:
5151
- --client-max-qps={{ .Values.args.clientQPS }}
5252
- --client-burst={{ .Values.args.clientBurst }}
5353
- --health-probe-bind-address=:8081
54-
{{- if and .Values.prometheus.enabled (eq .Values.kubernetesDistribution "OpenShift") }}
54+
{{- if and .Values.prometheus.enabled (eq .Values.hostingKubernetesDistribution "OpenShift") }}
5555
- --secure-metrics=true
5656
- --metrics-bind-address=0.0.0.0:8443
5757
{{- else if .Values.prometheus.enabled }}
@@ -114,7 +114,7 @@ spec:
114114
failureThreshold: 30
115115
periodSeconds: 10
116116
{{- end }}
117-
{{- if and .Values.prometheus.enabled (eq .Values.kubernetesDistribution "OpenShift") }}
117+
{{- if and .Values.prometheus.enabled (eq .Values.hostingKubernetesDistribution "OpenShift") }}
118118
ports:
119119
- name: metrics
120120
protocol: TCP
@@ -127,7 +127,7 @@ spec:
127127
{{- end }}
128128
resources: {{- toYaml .Values.resources | nindent 10 }}
129129
volumeMounts:
130-
{{- if and .Values.prometheus.enabled (eq .Values.kubernetesDistribution "OpenShift") }}
130+
{{- if and .Values.prometheus.enabled (eq .Values.hostingKubernetesDistribution "OpenShift") }}
131131
- mountPath: "/var/run/metrics-cert"
132132
name: metrics-cert
133133
readOnly: true
@@ -155,7 +155,7 @@ spec:
155155
secret:
156156
secretName: {{ .Values.managedKubeConfigSecret }}
157157
{{- end }}
158-
{{- if and .Values.prometheus.enabled (eq .Values.kubernetesDistribution "OpenShift") }}
158+
{{- if and .Values.prometheus.enabled (eq .Values.hostingKubernetesDistribution "OpenShift") }}
159159
- name: metrics-cert
160160
secret:
161161
secretName: {{ include "controller.fullname" . }}-metrics

‎pkg/addon/configpolicy/manifests/managedclusterchart/templates/install-namespace.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ metadata:
88
addon.open-cluster-management.io/namespace: "true"
99
addon.open-cluster-management.io/hosted-manifest-location: hosting
1010
{{- end }}
11-
{{- if and .Values.prometheus.enabled (eq .Values.kubernetesDistribution "OpenShift") }}
11+
{{- if and .Values.prometheus.enabled (eq .Values.hostingKubernetesDistribution "OpenShift") }}
1212
openshift.io/cluster-monitoring: "true"
1313
{{- end }}
1414
{{- if or (eq .Release.Namespace "open-cluster-management-agent-addon") (eq (.Release.Namespace | trimPrefix "klusterlet-") .Values.clusterName) }}

‎pkg/addon/configpolicy/manifests/managedclusterchart/templates/monitoring_role.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# Copyright Contributors to the Open Cluster Management project
22

3-
{{- if and .Values.prometheus.enabled (eq .Values.kubernetesDistribution "OpenShift") }}
3+
{{- if and .Values.prometheus.enabled (eq .Values.hostingKubernetesDistribution "OpenShift") }}
44
apiVersion: rbac.authorization.k8s.io/v1
55
kind: Role
66
metadata:

0 commit comments

Comments
 (0)