WIP: Default cloud.conf if no configmap is found

Signed-off-by: Nolan Brubaker <[email protected]>

Author: nrb

pkg/cloud/aws/aws_config_transformer.go

@@ -27,12 +27,15 @@ func CloudConfigTransformer(source string, infra *configv1.Infrastructure, netwo
 }
 
 func readAWSConfig(source string) (*awsconfig.CloudConfig, error) {
+	cfg := &awsconfig.CloudConfig{}
+
+	// There are cases in which a cloud config was not installed with a cluster, and this is valid.
+	// We'll want to return the cloud config to be modified for OpenShift defaults in that case.
 	if len(source) == 0 {
-		return nil, fmt.Errorf("empty INI file")
+		return cfg, nil
 	}
 
 	// Use the same method the AWS CCM uses to load configuration.
-	cfg := &awsconfig.CloudConfig{}
 	if err := gcfg.FatalOnly(gcfg.ReadStringInto(cfg, source)); err != nil {
 		return nil, fmt.Errorf("failed to parse INI file: %w", err)
 	}

pkg/cloud/cloud.go

@@ -25,6 +25,10 @@ import (
 // consumer of the transformer.
 type cloudConfigTransformer func(source string, infra *configv1.Infrastructure, network *configv1.Network) (string, error)
 
+// defaultCloudConfig returns a default cloud configuration in the event that a cluster does not have a source config map.
+// Clusters created prior to 4.14 do not always have a source config map.
+type defaultCloudConfig func(platform configv1.PlatformType) (string, error)
+
 // GetCloudConfigTransformer returns the function that should be used to transform
 // the cloud configuration config map, and a boolean to indicate if the config should
 // be synced from the CCO namespace before applying the transformation.

pkg/controllers/cloud_config_sync_controller.go

@@ -55,21 +55,6 @@ func (r *CloudConfigReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 		return ctrl.Result{}, err
 	}
 
-	syncNeeded, err := r.isCloudConfigSyncNeeded(infra.Status.PlatformStatus, infra.Spec.CloudConfig)
-	if err != nil {
-		if err := r.setDegradedCondition(ctx); err != nil {
-			return ctrl.Result{}, fmt.Errorf("failed to set conditions for cloud config controller: %v", err)
-		}
-		return ctrl.Result{}, err
-	}
-	if !syncNeeded {
-		if err := r.setAvailableCondition(ctx); err != nil {
-			return ctrl.Result{}, fmt.Errorf("failed to set conditions for cloud config controller: %v", err)
-		}
-		klog.Infof("cloud-config sync is not needed, returning early")
-		return ctrl.Result{}, nil
-	}
-
 	cloudConfigTransformerFn, needsManagedConfigLookup, err := cloud.GetCloudConfigTransformer(infra.Status.PlatformStatus)
 	if err != nil {
 		klog.Errorf("unable to get cloud config transformer function; unsupported platform")
@@ -113,15 +98,20 @@ func (r *CloudConfigReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 			Name:      infra.Spec.CloudConfig.Name,
 			Namespace: OpenshiftConfigNamespace,
 		}
-		if err := r.Get(ctx, openshiftUnmanagedCMKey, sourceCM); err != nil {
+		if err := r.Get(ctx, openshiftUnmanagedCMKey, sourceCM); err == nil {
+			return ctrl.Result{}, err
+		} else if errors.IsNotFound(err) {
+			klog.Warningf("unmanaged cloud-config is not found, falling back to default cloud config.")
+		} else {
 			klog.Errorf("unable to get cloud-config for sync")
 			if err := r.setDegradedCondition(ctx); err != nil {
 				return ctrl.Result{}, fmt.Errorf("failed to set conditions for cloud config controller: %v", err)
 			}
-			return ctrl.Result{}, err
 		}
 	}
 
+	klog.Infof("PAST NOT FOUND")
+
 	sourceCM, err = r.prepareSourceConfigMap(sourceCM, infra)
 	if err != nil {
 		if err := r.setDegradedCondition(ctx); err != nil {
@@ -130,6 +120,8 @@ func (r *CloudConfigReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 		return ctrl.Result{}, err
 	}
 
+	klog.Infof("PAST PREPARE")
+
 	if cloudConfigTransformerFn != nil {
 		// We ignore stuff in sourceCM.BinaryData. This isn't allowed to
 		// contain any key that overlaps with those found in sourceCM.Data and
@@ -144,6 +136,8 @@ func (r *CloudConfigReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 		sourceCM.Data[defaultConfigKey] = output
 	}
 
+	klog.Infof("PAST TRANSFORM")
+
 	targetCM := &corev1.ConfigMap{}
 	targetConfigMapKey := client.ObjectKey{
 		Namespace: r.ManagedNamespace,
@@ -183,27 +177,6 @@ func (r *CloudConfigReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 	return ctrl.Result{}, nil
 }
 
-func (r *CloudConfigReconciler) isCloudConfigSyncNeeded(platformStatus *configv1.PlatformStatus, infraCloudConfigRef configv1.ConfigMapFileReference) (bool, error) {
-	if platformStatus == nil {
-		return false, fmt.Errorf("platformStatus is required")
-	}
-	switch platformStatus.Type {
-	case configv1.AzurePlatformType,
-		configv1.GCPPlatformType,
-		configv1.VSpherePlatformType,
-		configv1.IBMCloudPlatformType,
-		configv1.PowerVSPlatformType,
-		configv1.OpenStackPlatformType,
-		configv1.NutanixPlatformType:
-		return true, nil
-	case configv1.AWSPlatformType:
-		// Some of AWS regions might require to sync a cloud-config, in such case reference in infra resource will be presented
-		return infraCloudConfigRef.Name != "", nil
-	default:
-		return false, nil
-	}
-}
-
 func (r *CloudConfigReconciler) prepareSourceConfigMap(source *corev1.ConfigMap, infra *configv1.Infrastructure) (*corev1.ConfigMap, error) {
 	// Keys might be different between openshift-config/cloud-config and openshift-config-managed/kube-cloud-config
 	// Always use "cloud.conf" which is default one across openshift
@@ -218,10 +191,11 @@ func (r *CloudConfigReconciler) prepareSourceConfigMap(source *corev1.ConfigMap,
 		delete(cloudConfCm.Data, infraConfigKey)
 		return cloudConfCm, nil
 	}
-	return nil, fmt.Errorf(
-		"key %s specified in infra resource does not found in source configmap %s",
-		infraConfigKey, client.ObjectKeyFromObject(source),
-	)
+
+	// If there was no relevant data in the configmap, make a default entry for it.
+	cloudConfCm.Data[defaultConfigKey] = ""
+
+	return cloudConfCm, nil
 }
 
 func (r *CloudConfigReconciler) isCloudConfigEqual(source *corev1.ConfigMap, target *corev1.ConfigMap) bool {

pkg/controllers/cloud_config_sync_controller_test.go

@@ -151,12 +151,14 @@ var _ = Describe("prepareSourceConfigMap reconciler method", func() {
 		Expect(reconciler.isCloudConfigEqual(preparedConfig, managedCloudConfig)).Should(BeTrue())
 	})
 
-	It("config preparation should fail if key from infra resource does not found", func() {
-		brokenInfraConfig := infraCloudConfig.DeepCopy()
-		brokenInfraConfig.Data = map[string]string{"hehehehehe": "bar"}
-		_, err := reconciler.prepareSourceConfigMap(brokenInfraConfig, infra)
-		Expect(err).Should(Not(Succeed()))
-		Expect(err.Error()).Should(BeEquivalentTo("key foo specified in infra resource does not found in source configmap openshift-config/test-config"))
+	It("config preparation should create required key if key isn't found in infrastructure", func() {
+		infraConfig := infraCloudConfig.DeepCopy()
+		infraConfig.Data = map[string]string{"hehehehehe": "bar"}
+		preparedConfig, err := reconciler.prepareSourceConfigMap(infraConfig, infra)
+		Expect(err).Should(Succeed())
+		_, ok := preparedConfig.Data[defaultConfigKey]
+		Expect(ok).Should(BeTrue())
+		Expect(len(preparedConfig.Data[defaultConfigKey])).Should(BeEquivalentTo(0))
 	})
 
 	It("config preparation should not touch extra fields in infra ConfigMap", func() {
@@ -467,7 +469,7 @@ var _ = Describe("Cloud config sync reconciler", func() {
 			Expect(cl.Create(ctx, makeInfraCloudConfig(configv1.AWSPlatformType))).To(Succeed())
 		})
 
-		It("should skip config sync for AWS platform if there is no reference in infra resource", func() {
+		It("should sync for a default config AWS platform if there is no reference in infra resource", func() {
 			infraResource := makeInfrastructureResource(configv1.AWSPlatformType)
 			infraResource.Spec.CloudConfig.Name = ""
 			Expect(cl.Create(ctx, infraResource)).To(Succeed())
@@ -481,7 +483,7 @@ var _ = Describe("Cloud config sync reconciler", func() {
 			allCMs := &corev1.ConfigMapList{}
 			Expect(cl.List(ctx, allCMs, &client.ListOptions{Namespace: targetNamespaceName})).To(Succeed())
 
-			Expect(len(allCMs.Items)).To(BeZero())
+			Expect(len(allCMs.Items)).To(BeEquivalentTo(1))
 		})
 
 		It("should perform config sync for AWS platform if there is a reference in infra resource", func() {