From eafc2703f7338741bdffd6246d699203de52a342 Mon Sep 17 00:00:00 2001 From: Enxebre Date: Mon, 22 Apr 2019 13:34:58 +0200 Subject: [PATCH 1/4] Disable machine health check by default --- pkg/operator/operator.go | 7 +-- pkg/operator/operator_test.go | 106 ++++++++++++++-------------------- pkg/operator/sync.go | 3 +- 3 files changed, 47 insertions(+), 69 deletions(-) diff --git a/pkg/operator/operator.go b/pkg/operator/operator.go index d9b1a1a2ef..70b7f84d08 100644 --- a/pkg/operator/operator.go +++ b/pkg/operator/operator.go @@ -223,10 +223,9 @@ func (optr *Operator) maoConfigFromInfrastructure() (*OperatorConfig, error) { return &OperatorConfig{ TargetNamespace: optr.namespace, Controllers: Controllers{ - Provider: providerControllerImage, - NodeLink: machineAPIOperatorImage, - MachineHealthCheck: machineAPIOperatorImage, - MachineHealthCheckEnabled: true, + Provider: providerControllerImage, + NodeLink: machineAPIOperatorImage, + MachineHealthCheck: machineAPIOperatorImage, }, }, nil } diff --git a/pkg/operator/operator_test.go b/pkg/operator/operator_test.go index efdde1b42b..a0a24a174d 100644 --- a/pkg/operator/operator_test.go +++ b/pkg/operator/operator_test.go @@ -181,21 +181,34 @@ func deploymentHasContainer(d *appsv1.Deployment, containerName string) bool { return false } -func TestOperatorSyncClusterAPIControllerHealthCheckControllerNotDeployed(t *testing.T) { - fg := &v1.FeatureGate{ - ObjectMeta: metav1.ObjectMeta{ - Name: MachineAPIFeatureGateName, +func TestOperatorSyncClusterAPIControllerHealthCheckController(t *testing.T) { + tests := []struct { + featureGate *v1.FeatureGate + expectedMachineHealthCheckController bool + }{{ + featureGate: &v1.FeatureGate{ + ObjectMeta: metav1.ObjectMeta{ + Name: MachineAPIFeatureGateName, + }, + Spec: v1.FeatureGateSpec{ + FeatureSet: configv1.Default, + }, }, - Spec: v1.FeatureGateSpec{ - FeatureSet: configv1.Default, + expectedMachineHealthCheckController: false, + }, { + featureGate: &v1.FeatureGate{}, + expectedMachineHealthCheckController: false, + }, { + featureGate: &v1.FeatureGate{ + ObjectMeta: metav1.ObjectMeta{ + Name: MachineAPIFeatureGateName, + }, + Spec: v1.FeatureGateSpec{ + FeatureSet: configv1.TechPreviewNoUpgrade, + }, }, - } - - kubeclientSet := fakekube.NewSimpleClientset() - op, err := initOperator(fg, kubeclientSet) - if err != nil { - t.Errorf("Unable to init operator: %v", err) - } + expectedMachineHealthCheckController: true, + }} oc := OperatorConfig{ TargetNamespace: targetNamespace, @@ -206,59 +219,24 @@ func TestOperatorSyncClusterAPIControllerHealthCheckControllerNotDeployed(t *tes }, } - if err := op.syncClusterAPIController(oc); err != nil { - t.Errorf("Failed to sync machine API controller: %v", err) - } - - d, err := kubeclientSet.AppsV1().Deployments(targetNamespace).Get(deploymentName, metav1.GetOptions{}) - if err != nil { - t.Errorf("Failed to get %q deployment: %v", deploymentName, err) - } - - if deploymentHasContainer(d, hcControllerName) { - t.Errorf("Did not expect to find %q container", hcControllerName) - } else { - t.Logf("%q container not found as expected", hcControllerName) - } -} - -func TestOperatorSyncClusterAPIControllerHealthCheckControllerDeployed(t *testing.T) { - fg := &v1.FeatureGate{ - ObjectMeta: metav1.ObjectMeta{ - Name: MachineAPIFeatureGateName, - }, - Spec: v1.FeatureGateSpec{ - FeatureSet: configv1.TechPreviewNoUpgrade, - }, - } - - kubeclientSet := fakekube.NewSimpleClientset() - op, err := initOperator(fg, kubeclientSet) - if err != nil { - t.Errorf("Unable to init operator: %v", err) - } - - oc := OperatorConfig{ - TargetNamespace: targetNamespace, - Controllers: Controllers{ - Provider: "controllers-provider", - NodeLink: "controllers-nodelink", - MachineHealthCheck: "controllers-machinehealthcheck", - }, - } + for _, tc := range tests { + kubeclientSet := fakekube.NewSimpleClientset() + op, err := initOperator(tc.featureGate, kubeclientSet) + if err != nil { + t.Fatalf("Unable to init operator: %v", err) + } - if err := op.syncClusterAPIController(oc); err != nil { - t.Errorf("Failed to sync machine API controller: %v", err) - } + if err := op.syncClusterAPIController(oc); err != nil { + t.Errorf("Failed to sync machine API controller: %v", err) + } - d, err := op.deployLister.Deployments(targetNamespace).Get(deploymentName) - if err != nil { - t.Errorf("Failed to get %q deployment: %v", deploymentName, err) - } + d, err := op.deployLister.Deployments(targetNamespace).Get(deploymentName) + if err != nil { + t.Errorf("Failed to get %q deployment: %v", deploymentName, err) + } - if deploymentHasContainer(d, hcControllerName) { - t.Logf("%q container found as expected", hcControllerName) - } else { - t.Errorf("Failed to find find %q container", hcControllerName) + if deploymentHasContainer(d, hcControllerName) != tc.expectedMachineHealthCheckController { + t.Errorf("Expected deploymentHasContainer for %q container to be %t", hcControllerName, tc.expectedMachineHealthCheckController) + } } } diff --git a/pkg/operator/sync.go b/pkg/operator/sync.go index 49e838bcdf..917557f67d 100644 --- a/pkg/operator/sync.go +++ b/pkg/operator/sync.go @@ -57,7 +57,7 @@ func (optr *Operator) syncClusterAPIController(config OperatorConfig) error { if !errors.IsNotFound(err) { return err } - glog.V(2).Infof("failed to find feature gate %s, will use default feature set", MachineAPIFeatureGateName) + glog.V(2).Infof("Failed to find feature gate %q, will use default feature set", MachineAPIFeatureGateName) featureSet = osev1.Default } else { featureSet = featureGate.Spec.FeatureSet @@ -70,6 +70,7 @@ func (optr *Operator) syncClusterAPIController(config OperatorConfig) error { // add machine-health-check controller container if it exists and enabled under feature gates if enabled, ok := features[FeatureGateMachineHealthCheck]; ok && enabled { + glog.V(2).Infof("Feature %q is enabled", FeatureGateMachineHealthCheck) config.Controllers.MachineHealthCheckEnabled = true } From 8e60ca31343c1647bac38577e10aa92f99fd539b Mon Sep 17 00:00:00 2001 From: Enxebre Date: Mon, 22 Apr 2019 17:02:51 +0200 Subject: [PATCH 2/4] Start ConfigInformerFactory and wait for cache to sync --- cmd/machine-api-operator/start.go | 1 + pkg/operator/operator.go | 9 +++++---- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/cmd/machine-api-operator/start.go b/cmd/machine-api-operator/start.go index 346788ccfd..cc79c55ae6 100644 --- a/cmd/machine-api-operator/start.go +++ b/cmd/machine-api-operator/start.go @@ -62,6 +62,7 @@ func runStartCmd(cmd *cobra.Command, args []string) { } ctrlCtx.KubeNamespacedInformerFactory.Start(ctrlCtx.Stop) + ctrlCtx.ConfigInformerFactory.Start(ctrlCtx.Stop) close(ctrlCtx.InformersStarted) select {} diff --git a/pkg/operator/operator.go b/pkg/operator/operator.go index 70b7f84d08..eaded36455 100644 --- a/pkg/operator/operator.go +++ b/pkg/operator/operator.go @@ -52,8 +52,8 @@ type Operator struct { deployLister appslisterv1.DeploymentLister deployListerSynced cache.InformerSynced - featureGateLister configlistersv1.FeatureGateLister - featureGateCacheSync cache.InformerSynced + featureGateLister configlistersv1.FeatureGateLister + featureGateCacheSynced cache.InformerSynced // queue only ever has one item, but it has nice error handling backoff/retry semantics queue workqueue.RateLimitingInterface @@ -108,7 +108,7 @@ func New( optr.deployListerSynced = deployInformer.Informer().HasSynced optr.featureGateLister = featureGateInformer.Lister() - optr.featureGateCacheSync = featureGateInformer.Informer().HasSynced + optr.featureGateCacheSynced = featureGateInformer.Informer().HasSynced return optr } @@ -122,7 +122,8 @@ func (optr *Operator) Run(workers int, stopCh <-chan struct{}) { defer glog.Info("Shutting down Machine API Operator") if !cache.WaitForCacheSync(stopCh, - optr.deployListerSynced) { + optr.deployListerSynced, + optr.featureGateCacheSynced) { glog.Error("Failed to sync caches") return } From d2152cd54e97a3844b084aeab38dae2dfcea9596 Mon Sep 17 00:00:00 2001 From: Enxebre Date: Mon, 22 Apr 2019 18:02:50 +0200 Subject: [PATCH 3/4] Add perms for get/list/watch featuregates --- install/0000_30_machine-api-operator_08_rbac.yaml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/install/0000_30_machine-api-operator_08_rbac.yaml b/install/0000_30_machine-api-operator_08_rbac.yaml index 1145ac46d5..cb671b7f0c 100644 --- a/install/0000_30_machine-api-operator_08_rbac.yaml +++ b/install/0000_30_machine-api-operator_08_rbac.yaml @@ -50,6 +50,16 @@ rules: verbs: - get + - apiGroups: + - config.openshift.io + resources: + - featuregates + - featuregates/status + verbs: + - get + - list + - watch + - apiGroups: - metalkube.org resources: From 1f46dde7688a1c06acc5e4132125a1272f11d75b Mon Sep 17 00:00:00 2001 From: Jan Chaloupka Date: Tue, 23 Apr 2019 09:10:38 +0200 Subject: [PATCH 4/4] kubemark: Deploy feature gate CRDs --- ...05_config-operator_01_featuregate.crd.yaml | 49 +++++++++++++++++++ config/machine-api-operator-patch.yaml | 10 ++++ kustomization.yaml | 3 ++ 3 files changed, 62 insertions(+) create mode 100644 config/0000_05_config-operator_01_featuregate.crd.yaml diff --git a/config/0000_05_config-operator_01_featuregate.crd.yaml b/config/0000_05_config-operator_01_featuregate.crd.yaml new file mode 100644 index 0000000000..fcb2bc1826 --- /dev/null +++ b/config/0000_05_config-operator_01_featuregate.crd.yaml @@ -0,0 +1,49 @@ +apiVersion: apiextensions.k8s.io/v1beta1 +kind: CustomResourceDefinition +metadata: + name: featuregates.config.openshift.io +spec: + group: config.openshift.io + version: v1 + scope: Cluster + names: + kind: FeatureGate + singular: featuregate + plural: featuregates + listKind: FeatureGateList + versions: + - name: v1 + served: true + storage: true + validation: + openAPIV3Schema: + properties: + apiVersion: + description: 'APIVersion defines the versioned schema of this representation + of an object. Servers should convert recognized schemas to the latest + internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/api-conventions.md#resources' + type: string + kind: + description: 'Kind is a string value representing the REST resource this + object represents. Servers may infer this from the endpoint the client + submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/api-conventions.md#types-kinds' + type: string + metadata: + description: Standard object's metadata. + type: object + spec: + description: spec holds user settable values for configuration + properties: + featureSet: + description: featureSet changes the list of features in the cluster. The + default is empty. Be very careful adjusting this setting. Turning + on or off features may cause irreversible changes in your cluster + which cannot be undone. + type: string + type: object + status: + description: status holds observed values from the cluster. They may not + be overridden. + type: object + required: + - spec diff --git a/config/machine-api-operator-patch.yaml b/config/machine-api-operator-patch.yaml index 6d09f1e2c2..2b26f37313 100644 --- a/config/machine-api-operator-patch.yaml +++ b/config/machine-api-operator-patch.yaml @@ -44,6 +44,16 @@ rules: verbs: - get + - apiGroups: + - config.openshift.io + resources: + - featuregates + - featuregates/status + verbs: + - get + - list + - watch + - apiGroups: - metalkube.org resources: diff --git a/kustomization.yaml b/kustomization.yaml index 293c0fc292..829017d4d6 100644 --- a/kustomization.yaml +++ b/kustomization.yaml @@ -11,6 +11,9 @@ resources: # Pulled from https://github.com/openshift/cluster-version-operator/blob/master/install/0000_00_cluster-version-operator_01_clusteroperator.crd.yaml # and updated (delete depricated clusteroperators.operatorstatus.openshift.io CRD) - config/0000_00_cluster-version-operator_01_clusteroperator.crd.yaml +# Install feature gate CRD reguired by mao (so it can enable/disable features) +# Pulled from https://raw.githubusercontent.com/openshift/cluster-config-operator/master/manifests/0000_05_config-operator_01_featuregate.crd.yaml +- config/0000_05_config-operator_01_featuregate.crd.yaml # Install mao namespaces, CRDS and other resources to properly deploy machine API stack - install/0000_30_machine-api-operator_00_namespace.yaml - install/0000_30_machine-api-operator_01_images.configmap.yaml