Skip to content

Commit

Permalink
Merge pull request #293 from enxebre/feature-gate-fix
Browse files Browse the repository at this point in the history
fix feature gate
  • Loading branch information
openshift-merge-robot authored Apr 23, 2019
2 parents e94611f + 1f46dde commit dd8d2cf
Show file tree
Hide file tree
Showing 8 changed files with 125 additions and 73 deletions.
1 change: 1 addition & 0 deletions cmd/machine-api-operator/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {}
Expand Down
49 changes: 49 additions & 0 deletions config/0000_05_config-operator_01_featuregate.crd.yaml
Original file line number Diff line number Diff line change
@@ -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
10 changes: 10 additions & 0 deletions config/machine-api-operator-patch.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,16 @@ rules:
verbs:
- get

- apiGroups:
- config.openshift.io
resources:
- featuregates
- featuregates/status
verbs:
- get
- list
- watch

- apiGroups:
- metalkube.org
resources:
Expand Down
10 changes: 10 additions & 0 deletions install/0000_30_machine-api-operator_08_rbac.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,16 @@ rules:
verbs:
- get

- apiGroups:
- config.openshift.io
resources:
- featuregates
- featuregates/status
verbs:
- get
- list
- watch

- apiGroups:
- metalkube.org
resources:
Expand Down
3 changes: 3 additions & 0 deletions kustomization.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 8 additions & 8 deletions pkg/operator/operator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand All @@ -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
}
Expand Down Expand Up @@ -223,10 +224,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
}
106 changes: 42 additions & 64 deletions pkg/operator/operator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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)
}
}
}
3 changes: 2 additions & 1 deletion pkg/operator/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}

Expand Down

0 comments on commit dd8d2cf

Please sign in to comment.