Bug 1840782: Update conditions under which metal3 pod would be deployed

pkg/operator/baremetal_config.go

@@ -5,7 +5,6 @@ import (
 	"fmt"
 	"net"
 
-	"github.com/golang/glog"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -14,6 +13,7 @@ import (
 )
 
 var provisioningGVR = schema.GroupVersionResource{Group: "metal3.io", Resource: "provisionings", Version: "v1alpha1"}
+var provisioningGR = schema.GroupResource{Group: "metal3.io", Resource: "provisionings"}
 
 const (
 	baremetalProvisioningCR        = "provisioning-configuration"
@@ -48,47 +48,47 @@ func reportError(found bool, err error, configItem string, configName string) er
 	return fmt.Errorf("Unknown Error while reading %s from Baremetal provisioning CR %s", configItem, configName)
 }
 
-func getBaremetalProvisioningConfig(dc dynamic.Interface, configName string) (BaremetalProvisioningConfig, error) {
+func getBaremetalProvisioningConfig(dc dynamic.Interface, configName string) (*BaremetalProvisioningConfig, error) {
 	provisioningClient := dc.Resource(provisioningGVR)
 	provisioningConfig, err := provisioningClient.Get(context.Background(), configName, metav1.GetOptions{})
 	if apierrors.IsNotFound(err) {
-		glog.V(3).Infof("Baremetal provisioning CR %s is not found", configName)
-		return BaremetalProvisioningConfig{}, nil
+		// The provisioning CR is not present and that is not considered an error.
+		return nil, nil
 	}
 	if err != nil {
-		return BaremetalProvisioningConfig{}, reportError(true, err, "provisioning configuration", configName)
+		return nil, reportError(true, err, "provisioning configuration", configName)
 	}
 	provisioningSpec, found, err := unstructured.NestedMap(provisioningConfig.UnstructuredContent(), "spec")
 	if !found || err != nil {
-		return BaremetalProvisioningConfig{}, reportError(found, err, "Spec field", configName)
+		return nil, reportError(found, err, "Spec field", configName)
 	}
 	provisioningInterface, found, err := unstructured.NestedString(provisioningSpec, "provisioningInterface")
 	if !found || err != nil {
-		return BaremetalProvisioningConfig{}, reportError(found, err, "provisioningInterface", configName)
+		return nil, reportError(found, err, "provisioningInterface", configName)
 	}
 	provisioningIP, found, err := unstructured.NestedString(provisioningSpec, "provisioningIP")
 	if !found || err != nil {
-		return BaremetalProvisioningConfig{}, reportError(found, err, "provisioningIP", configName)
+		return nil, reportError(found, err, "provisioningIP", configName)
 	}
 	provisioningDHCPRange, found, err := unstructured.NestedString(provisioningSpec, "provisioningDHCPRange")
 	if !found || err != nil {
-		return BaremetalProvisioningConfig{}, reportError(found, err, "provisioningDHCPRange", configName)
+		return nil, reportError(found, err, "provisioningDHCPRange", configName)
 	}
 	provisioningOSDownloadURL, found, err := unstructured.NestedString(provisioningSpec, "provisioningOSDownloadURL")
 	if !found || err != nil {
-		return BaremetalProvisioningConfig{}, reportError(found, err, "provisioningOSDownloadURL", configName)
+		return nil, reportError(found, err, "provisioningOSDownloadURL", configName)
 	}
 	// If provisioningNetwork is not provided, set its value based on provisioningDHCPExternal
 	provisioningNetwork, foundNetworkState, err := unstructured.NestedString(provisioningSpec, "provisioningNetwork")
 	if err != nil {
-		return BaremetalProvisioningConfig{}, reportError(true, err, "provisioningNetwork", configName)
+		return nil, reportError(true, err, "provisioningNetwork", configName)
 	}
 	if !foundNetworkState {
 		// Check if provisioningDHCPExternal is present in the config
 		provisioningDHCPExternal, foundDHCP, err := unstructured.NestedBool(provisioningSpec, "provisioningDHCPExternal")
 		if !foundDHCP || err != nil {
 			// Both the new provisioningNetwork and the old provisioningDHCPExternal configs are not found.
-			return BaremetalProvisioningConfig{}, reportError(foundDHCP, err, "provisioningNetwork and provisioningDHCPExternal", configName)
+			return nil, reportError(foundDHCP, err, "provisioningNetwork and provisioningDHCPExternal", configName)
 		}
 		if !provisioningDHCPExternal {
 			provisioningNetwork = provisioningNetworkManaged
@@ -100,23 +100,24 @@ func getBaremetalProvisioningConfig(dc dynamic.Interface, configName string) (Ba
 	// The CIDR of the network needs to be extracted to form the provisioningIPCIDR
 	provisioningNetworkCIDR, found, err := unstructured.NestedString(provisioningSpec, "provisioningNetworkCIDR")
 	if !found || err != nil {
-		return BaremetalProvisioningConfig{}, reportError(found, err, "provisioningNetworkCIDR", configName)
+		return nil, reportError(found, err, "provisioningNetworkCIDR", configName)
 	}
 	// Check if the other config values make sense for the provisioningNetwork configured.
 	if provisioningInterface == "" && provisioningNetwork == provisioningNetworkManaged {
-		return BaremetalProvisioningConfig{}, fmt.Errorf("provisioningInterface cannot be empty when provisioningNetwork is Managed.")
+		return nil, fmt.Errorf("provisioningInterface cannot be empty when provisioningNetwork is Managed.")
 	}
 	if provisioningDHCPRange == "" && provisioningNetwork == provisioningNetworkManaged {
-		return BaremetalProvisioningConfig{}, fmt.Errorf("provisioningDHCPRange cannot be empty when provisioningNetwork is Managed or when the DHCP server needs to run with the metal3 cluster.")
+		return nil, fmt.Errorf("provisioningDHCPRange cannot be empty when provisioningNetwork is Managed or when the DHCP server needs to run with the metal3 cluster.")
 	}
-	return BaremetalProvisioningConfig{
+	baremetalConfig := BaremetalProvisioningConfig{
 		ProvisioningInterface:     provisioningInterface,
 		ProvisioningIp:            provisioningIP,
 		ProvisioningNetworkCIDR:   provisioningNetworkCIDR,
 		ProvisioningDHCPRange:     provisioningDHCPRange,
 		ProvisioningOSDownloadURL: provisioningOSDownloadURL,
 		ProvisioningNetwork:       provisioningNetwork,
-	}, nil
+	}
+	return &baremetalConfig, nil
 }
 
 func getProvisioningIPCIDR(baremetalConfig BaremetalProvisioningConfig) *string {

pkg/operator/baremetal_pod.go

@@ -18,7 +18,6 @@ import (
 )
 
 const (
-	baremetalConfigmap         = "metal3-config"
 	baremetalSharedVolume      = "metal3-shared"
 	baremetalSecretName        = "metal3-mariadb-password"
 	baremetalSecretKey         = "password"

pkg/operator/baremetal_test.go

@@ -5,6 +5,7 @@ import (
 
 	. "github.com/onsi/gomega"
 	"golang.org/x/net/context"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -186,10 +187,9 @@ func TestGetIncorrectBaremetalProvisioningCR(t *testing.T) {
 	}
 	dynamicClient := fakedynamic.NewSimpleDynamicClient(runtime.NewScheme(), u)
 	baremetalConfig, err := getBaremetalProvisioningConfig(dynamicClient, incorrectConfigResource)
-	if err != nil {
+	if err == nil && baremetalConfig == nil {
 		t.Logf("Unable to get Baremetal Provisioning Config from CR %s as expected", incorrectConfigResource)
-	}
-	if baremetalConfig.ProvisioningInterface != "" {
+	} else {
 		t.Errorf("BaremetalProvisioningConfig is not expected to be set.")
 	}
 }
@@ -206,13 +206,13 @@ func TestGetMetal3DeploymentConfig(t *testing.T) {
 		t.Logf("Unstructed Config:  %+v", u)
 		t.Errorf("Failed to get Baremetal Provisioning Config from CR %s", testConfigResource)
 	}
-	actualCacheURL := getMetal3DeploymentConfig("CACHEURL", baremetalConfig)
+	actualCacheURL := getMetal3DeploymentConfig("CACHEURL", *baremetalConfig)
 	if actualCacheURL != nil {
 		t.Errorf("CacheURL is found to be %s. CACHEURL is not expected.", *actualCacheURL)
 	} else {
 		t.Logf("CacheURL is not available as expected.")
 	}
-	actualOSImageURL := getMetal3DeploymentConfig("RHCOS_IMAGE_URL", baremetalConfig)
+	actualOSImageURL := getMetal3DeploymentConfig("RHCOS_IMAGE_URL", *baremetalConfig)
 	if actualOSImageURL != nil {
 		t.Logf("Actual OS Image Download URL is %s, Expected is %s", *actualOSImageURL, expectedOSImageURL)
 		if *actualOSImageURL != expectedOSImageURL {
@@ -221,7 +221,7 @@ func TestGetMetal3DeploymentConfig(t *testing.T) {
 	} else {
 		t.Errorf("OS Image Download URL is not available.")
 	}
-	actualProvisioningIPCIDR := getMetal3DeploymentConfig("PROVISIONING_IP", baremetalConfig)
+	actualProvisioningIPCIDR := getMetal3DeploymentConfig("PROVISIONING_IP", *baremetalConfig)
 	if actualProvisioningIPCIDR != nil {
 		t.Logf("Actual ProvisioningIP with CIDR is %s, Expected is %s", *actualProvisioningIPCIDR, expectedProvisioningIPCIDR)
 		if *actualProvisioningIPCIDR != expectedProvisioningIPCIDR {
@@ -230,7 +230,7 @@ func TestGetMetal3DeploymentConfig(t *testing.T) {
 	} else {
 		t.Errorf("Provisioning IP with CIDR is not available.")
 	}
-	actualProvisioningInterface := getMetal3DeploymentConfig("PROVISIONING_INTERFACE", baremetalConfig)
+	actualProvisioningInterface := getMetal3DeploymentConfig("PROVISIONING_INTERFACE", *baremetalConfig)
 	if actualProvisioningInterface != nil {
 		t.Logf("Actual Provisioning Interface is %s, Expected is %s", *actualProvisioningInterface, expectedProvisioningInterface)
 		if *actualProvisioningInterface != expectedProvisioningInterface {
@@ -239,7 +239,7 @@ func TestGetMetal3DeploymentConfig(t *testing.T) {
 	} else {
 		t.Errorf("Provisioning Interface is not available.")
 	}
-	actualDeployKernelURL := getMetal3DeploymentConfig("DEPLOY_KERNEL_URL", baremetalConfig)
+	actualDeployKernelURL := getMetal3DeploymentConfig("DEPLOY_KERNEL_URL", *baremetalConfig)
 	if actualDeployKernelURL != nil {
 		t.Logf("Actual Deploy Kernel URL is %s, Expected is %s", *actualDeployKernelURL, expectedDeployKernelURL)
 		if *actualDeployKernelURL != expectedDeployKernelURL {
@@ -248,7 +248,7 @@ func TestGetMetal3DeploymentConfig(t *testing.T) {
 	} else {
 		t.Errorf("Deploy Kernel URL is not available.")
 	}
-	actualDeployRamdiskURL := getMetal3DeploymentConfig("DEPLOY_RAMDISK_URL", baremetalConfig)
+	actualDeployRamdiskURL := getMetal3DeploymentConfig("DEPLOY_RAMDISK_URL", *baremetalConfig)
 	if actualDeployRamdiskURL != nil {
 		t.Logf("Actual Deploy Ramdisk URL is %s, Expected is %s", *actualDeployRamdiskURL, expectedDeployRamdiskURL)
 		if *actualDeployRamdiskURL != expectedDeployRamdiskURL {
@@ -257,7 +257,7 @@ func TestGetMetal3DeploymentConfig(t *testing.T) {
 	} else {
 		t.Errorf("Deploy Ramdisk URL is not available.")
 	}
-	actualIronicEndpoint := getMetal3DeploymentConfig("IRONIC_ENDPOINT", baremetalConfig)
+	actualIronicEndpoint := getMetal3DeploymentConfig("IRONIC_ENDPOINT", *baremetalConfig)
 	if actualIronicEndpoint != nil {
 		t.Logf("Actual Ironic Endpoint is %s, Expected is %s", *actualIronicEndpoint, expectedIronicEndpoint)
 		if *actualIronicEndpoint != expectedIronicEndpoint {
@@ -266,7 +266,7 @@ func TestGetMetal3DeploymentConfig(t *testing.T) {
 	} else {
 		t.Errorf("Ironic Endpoint is not available.")
 	}
-	actualIronicInspectorEndpoint := getMetal3DeploymentConfig("IRONIC_INSPECTOR_ENDPOINT", baremetalConfig)
+	actualIronicInspectorEndpoint := getMetal3DeploymentConfig("IRONIC_INSPECTOR_ENDPOINT", *baremetalConfig)
 	if actualIronicInspectorEndpoint != nil {
 		t.Logf("Actual Ironic Inspector Endpoint is %s, Expected is %s", *actualIronicInspectorEndpoint, expectedIronicInspectorEndpoint)
 		if *actualIronicInspectorEndpoint != expectedIronicInspectorEndpoint {
@@ -275,12 +275,12 @@ func TestGetMetal3DeploymentConfig(t *testing.T) {
 	} else {
 		t.Errorf("Ironic Inspector Endpoint is not available.")
 	}
-	actualHttpPort := getMetal3DeploymentConfig("HTTP_PORT", baremetalConfig)
+	actualHttpPort := getMetal3DeploymentConfig("HTTP_PORT", *baremetalConfig)
 	t.Logf("Actual Http Port is %s, Expected is %s", *actualHttpPort, expectedHttpPort)
 	if *actualHttpPort != expectedHttpPort {
 		t.Errorf("Actual %s and Expected %s Http Ports do not match", *actualHttpPort, expectedHttpPort)
 	}
-	actualDHCPRange := getMetal3DeploymentConfig("DHCP_RANGE", baremetalConfig)
+	actualDHCPRange := getMetal3DeploymentConfig("DHCP_RANGE", *baremetalConfig)
 	if actualDHCPRange != nil {
 		t.Logf("Actual DHCP Range is %s, Expected is %s", *actualDHCPRange, expectedProvisioningDHCPRange)
 		if *actualDHCPRange != expectedProvisioningDHCPRange {
@@ -334,10 +334,48 @@ func TestBaremetalProvisionigConfig(t *testing.T) {
 			g.Expect(err).To(BeNil())
 
 			for i, envVar := range configNames {
-				actualConfigValue := getMetal3DeploymentConfig(envVar, baremetalConfig)
+				actualConfigValue := getMetal3DeploymentConfig(envVar, *baremetalConfig)
 
 				g.Expect(*actualConfigValue).To(Equal(tc.expectedConfigValues[i]))
 			}
 		})
 	}
 }
+
+func TestSyncBaremetalControllers(t *testing.T) {
+	operatorConfig := newOperatorWithBaremetalConfig()
+
+	stop := make(chan struct{})
+	defer close(stop)
+	optr := newFakeOperator(nil, nil, stop)
+
+	u := &unstructured.Unstructured{Object: map[string]interface{}{}}
+	if err := yaml.Unmarshal([]byte(yamlContent), &u); err != nil {
+		t.Errorf("failed to unmarshall input yaml content:%v", err)
+	}
+	dynamicClient := fakedynamic.NewSimpleDynamicClient(runtime.NewScheme(), u)
+
+	optr.dynamicClient = dynamicClient
+
+	testCases := []struct {
+		name          string
+		configCRName  string
+		expectedError error
+	}{
+		{
+			name:          "InvalidCRName",
+			configCRName:  "baremetal-disabled",
+			expectedError: apierrors.NewNotFound(provisioningGR, "baremetal-disabled"),
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			g := NewWithT(t)
+
+			err := optr.syncBaremetalControllers(operatorConfig, tc.configCRName)
+			g.Expect(err).To(Equal(tc.expectedError))
+
+		})
+	}
+}

pkg/operator/sync.go

@@ -82,7 +82,13 @@ func (optr *Operator) syncAll(config *OperatorConfig) error {
 	// In addition, if the Provider is BareMetal, then bring up
 	// the baremetal-operator pod
 	if config.BaremetalControllers.BaremetalOperator != "" {
-		if err := optr.syncBaremetalControllers(config); err != nil {
+		err := optr.syncBaremetalControllers(config, baremetalProvisioningCR)
+		switch true {
+		case apierrors.IsNotFound(err):
+			glog.V(3).Info("Provisioning configuration not found. Skipping metal3 deployment.")
+		case err == nil:
+			glog.V(3).Info("Synced up all metal3 components")
+		case err != nil:
 			if err := optr.statusDegraded(err.Error()); err != nil {
 				// Just log the error here.  We still want to
 				// return the outer error.
@@ -91,7 +97,6 @@ func (optr *Operator) syncAll(config *OperatorConfig) error {
 			glog.Errorf("Error syncing metal3-controller: %v", err)
 			return err
 		}
-		glog.V(3).Info("Synced up all metal3 components")
 	}
 
 	if err := optr.statusAvailable(); err != nil {
@@ -192,9 +197,12 @@ func (optr *Operator) syncMutatingWebhook() error {
 	return nil
 }
 
-func (optr *Operator) syncBaremetalControllers(config *OperatorConfig) error {
+func (optr *Operator) syncBaremetalControllers(config *OperatorConfig, configName string) error {
 	// Try to get baremetal provisioning config from a CR
-	baremetalProvisioningConfig, err := getBaremetalProvisioningConfig(optr.dynamicClient, baremetalProvisioningCR)
+	baremetalProvisioningConfig, err := getBaremetalProvisioningConfig(optr.dynamicClient, configName)
+	if err == nil && baremetalProvisioningConfig == nil {
+		return apierrors.NewNotFound(provisioningGR, configName)
+	}
 	if err != nil {
 		return err
 	}
@@ -204,7 +212,7 @@ func (optr *Operator) syncBaremetalControllers(config *OperatorConfig) error {
 		return err
 	}
 
-	metal3Deployment := newMetal3Deployment(config, baremetalProvisioningConfig)
+	metal3Deployment := newMetal3Deployment(config, *baremetalProvisioningConfig)
 	expectedGeneration := resourcemerge.ExpectedDeploymentGeneration(metal3Deployment, optr.generations)
 	d, updated, err := resourceapply.ApplyDeployment(optr.kubeClient.AppsV1(),
 		events.NewLoggingEventRecorder(optr.name), metal3Deployment, expectedGeneration)