Merge pull request #11003 from seh/automated-cherry-pick-of-#11002-or…

…igin-release-1.20 Automated cherry pick of #11002: Honor an OpenStack StorageClass management choice
kubernetes · Mar 9, 2021 · a496fff · a496fff
2 parents 3266d8c + eece2ce
commit a496fff
Show file tree

Hide file tree

Showing 4 changed files with 140 additions and 5 deletions.
diff --git a/pkg/apis/kops/validation/validation_test.go b/pkg/apis/kops/validation/validation_test.go
@@ -1066,7 +1066,7 @@ func Test_Validate_CloudConfiguration(t *testing.T) {
 				}},
 		},
 		{
-			Description: "os false",
+			Description: "os true",
 			Input: kops.CloudConfiguration{
 				Openstack: &kops.OpenstackConfiguration{
 					BlockStorage: &kops.OpenstackBlockStorageConfig{

diff --git a/pkg/model/components/BUILD.bazel b/pkg/model/components/BUILD.bazel
@@ -49,6 +49,7 @@ go_library(
 go_test(
     name = "go_default_test",
     srcs = [
+        "cloudconfiguration_test.go",
         "containerd_test.go",
         "image_test.go",
         "kubecontrollermanager_test.go",
@@ -60,6 +61,7 @@ go_test(
         "//pkg/apis/kops:go_default_library",
         "//pkg/apis/kops/util:go_default_library",
         "//pkg/assets:go_default_library",
+        "//upup/pkg/fi:go_default_library",
         "//util/pkg/vfs:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
     ],

diff --git a/pkg/model/components/cloudconfiguration.go b/pkg/model/components/cloudconfiguration.go
@@ -37,12 +37,20 @@ func (b *CloudConfigurationOptionsBuilder) BuildOptions(o interface{}) error {
 		clusterSpec.CloudConfig = c
 	}
 
-	if c.ManageStorageClasses == nil {
-		c.ManageStorageClasses = fi.Bool(true)
-	}
-
 	// NB: See file openstack.go for establishing default values for the CloudConfig.Openstack
 	// field.
 
+	if c.ManageStorageClasses == nil {
+		var manage *bool
+		if c.Openstack != nil && c.Openstack.BlockStorage != nil && c.Openstack.BlockStorage.CreateStorageClass != nil {
+			// Avoid a spurious conflict with a user-specified configuration for OpenStack by
+			// adopting that more particular setting generally.
+			manage = c.Openstack.BlockStorage.CreateStorageClass
+		} else {
+			manage = fi.Bool(true)
+		}
+		c.ManageStorageClasses = manage
+	}
+
 	return nil
 }
diff --git a/pkg/model/components/cloudconfiguration_test.go b/pkg/model/components/cloudconfiguration_test.go
@@ -0,0 +1,125 @@
+/*
+Copyright 2021 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package components
+
+import (
+	"testing"
+
+	kopsapi "k8s.io/kops/pkg/apis/kops"
+	"k8s.io/kops/upup/pkg/fi"
+)
+
+func TestCloudConfigurationOptionsBuilder(t *testing.T) {
+	ob := &CloudConfigurationOptionsBuilder{
+		Context: nil,
+	}
+	disabled := fi.Bool(false)
+	enabled := fi.Bool(true)
+	for _, test := range []struct {
+		description              string
+		generalManageSCs         *bool
+		openStackManageSCs       *bool
+		expectedGeneralManageSCs *bool
+	}{
+		{
+			"neither",
+			nil,
+			nil,
+			enabled,
+		},
+		{
+			"all false",
+			disabled,
+			nil,
+			disabled,
+		},
+		{
+			"all true",
+			enabled,
+			nil,
+			enabled,
+		},
+		{
+			"os false",
+			nil,
+			disabled,
+			disabled,
+		},
+		{
+			"os true",
+			nil,
+			enabled,
+			enabled,
+		},
+		{
+			"all false, os false",
+			disabled,
+			disabled,
+			disabled,
+		},
+		{
+			"all false, os true",
+			// Caught as conflict during validation.
+			disabled,
+			enabled,
+			disabled,
+		},
+		{
+			"all true, os false",
+			// Caught as conflict during validation.
+			enabled,
+			disabled,
+			enabled,
+		},
+		{
+			"all true, os true",
+			enabled,
+			enabled,
+			enabled,
+		},
+	} {
+		t.Run(test.description, func(t *testing.T) {
+			spec := kopsapi.ClusterSpec{
+				CloudProvider: string(kopsapi.CloudProviderOpenstack),
+				CloudConfig: &kopsapi.CloudConfiguration{
+					Openstack: &kopsapi.OpenstackConfiguration{
+						BlockStorage: &kopsapi.OpenstackBlockStorageConfig{},
+					},
+				},
+			}
+			if p := test.generalManageSCs; p != nil {
+				spec.CloudConfig.ManageStorageClasses = p
+			}
+			if p := test.openStackManageSCs; p != nil {
+				spec.CloudConfig.Openstack.BlockStorage.CreateStorageClass = p
+			}
+			if err := ob.BuildOptions(&spec); err != nil {
+				t.Fatalf("failed to build options: %v", err)
+			}
+			if want, got := test.expectedGeneralManageSCs, spec.CloudConfig.ManageStorageClasses; (want == nil) != (got == nil) || (got != nil && *got != *want) {
+				switch {
+				case want == nil:
+					t.Errorf("spec.cloudConfig.manageStorageClasses: want nil, got %t", *got)
+				case got == nil:
+					t.Errorf("spec.cloudConfig.manageStorageClasses: want %t, got nil", *want)
+				default:
+					t.Errorf("spec.cloudConfig.manageStorageClasses: want %t, got %t", *want, *got)
+				}
+			}
+		})
+	}
+}