From a10acb9f0da22fc013129ec71408f0b8d93c350f Mon Sep 17 00:00:00 2001 From: Sandhya Dasu Date: Mon, 17 Aug 2020 11:44:58 -0400 Subject: [PATCH] Bug 1840782: Update conditions under which metal3 pod would be deployed Changes to accommodate Baremetal UPI deployments where the platform type Baremetal without a need to deploy the metal3 pod. --- pkg/operator/baremetal_config.go | 35 +++++++++-------- pkg/operator/baremetal_pod.go | 1 - pkg/operator/baremetal_test.go | 66 +++++++++++++++++++++++++------- pkg/operator/sync.go | 18 ++++++--- 4 files changed, 83 insertions(+), 37 deletions(-) diff --git a/pkg/operator/baremetal_config.go b/pkg/operator/baremetal_config.go index 73f3d417cc..f424c85d56 100644 --- a/pkg/operator/baremetal_config.go +++ b/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 { diff --git a/pkg/operator/baremetal_pod.go b/pkg/operator/baremetal_pod.go index 83ba5445fe..1a2e84d074 100644 --- a/pkg/operator/baremetal_pod.go +++ b/pkg/operator/baremetal_pod.go @@ -18,7 +18,6 @@ import ( ) const ( - baremetalConfigmap = "metal3-config" baremetalSharedVolume = "metal3-shared" baremetalSecretName = "metal3-mariadb-password" baremetalSecretKey = "password" diff --git a/pkg/operator/baremetal_test.go b/pkg/operator/baremetal_test.go index 61f71c34d4..8d17663604 100644 --- a/pkg/operator/baremetal_test.go +++ b/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)) + + }) + } +} diff --git a/pkg/operator/sync.go b/pkg/operator/sync.go index 258f3a7e95..48ba1ef98a 100644 --- a/pkg/operator/sync.go +++ b/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)