diff --git a/apiserver/facades/client/application/application.go b/apiserver/facades/client/application/application.go index edcf4b57dd1..f6e709bf340 100644 --- a/apiserver/facades/client/application/application.go +++ b/apiserver/facades/client/application/application.go @@ -23,7 +23,6 @@ import ( apiservererrors "github.com/juju/juju/apiserver/errors" "github.com/juju/juju/apiserver/facade" "github.com/juju/juju/caas" - k8s "github.com/juju/juju/caas/kubernetes/provider" k8sconstants "github.com/juju/juju/caas/kubernetes/provider/constants" corebase "github.com/juju/juju/core/base" corecharm "github.com/juju/juju/core/charm" @@ -102,7 +101,6 @@ type APIBase struct { type CaasBrokerInterface interface { ValidateStorageClass(ctx context.Context, config map[string]interface{}) error - Version() (*version.Number, error) } func newFacadeBase(stdCtx context.Context, ctx facade.ModelContext) (*APIBase, error) { @@ -454,23 +452,17 @@ func (c caasDeployParams) precheck( return fmt.Errorf("getting model config: %w", err) } - // For older charms, operator-storage model config is mandatory. - if k8s.RequireOperatorStorage(c.charm) { - storageClassName, _ := cfg.AllAttrs()[k8sconstants.OperatorStorageKey].(string) - if storageClassName == "" { - return errors.New( - "deploying this Kubernetes application requires a suitable storage class.\n" + - "None have been configured. Set the operator-storage model config to " + - "specify which storage class should be used to allocate operator storage.\n" + - "See https://discourse.charmhub.io/t/getting-started/152.", - ) + workloadStorageClass, _ := cfg.AllAttrs()[k8sconstants.WorkloadStorageKey].(string) + for storageName, cons := range c.storage { + if cons.Pool == "" && workloadStorageClass == "" { + return errors.Errorf("storage pool for %q must be specified since there's no model default storage class", storageName) } - sp, err := charmStorageParams(ctx, "", storageClassName, cfg, "", storageService, registry) + sp, err := charmStorageParams(ctx, "", workloadStorageClass, cfg, cons.Pool, storageService, registry) if err != nil { - return errors.Annotatef(err, "getting operator storage params for %q", c.applicationName) + return errors.Annotatef(err, "getting workload storage params for %q", c.applicationName) } if sp.Provider != string(k8sconstants.StorageProviderType) { - poolName := cfg.AllAttrs()[k8sconstants.OperatorStorageKey] + poolName := cfg.AllAttrs()[k8sconstants.WorkloadStorageKey] return errors.Errorf( "the %q storage pool requires a provider type of %q, not %q", poolName, k8sconstants.StorageProviderType, sp.Provider) } @@ -479,17 +471,6 @@ func (c caasDeployParams) precheck( } } - workloadStorageClass, _ := cfg.AllAttrs()[k8sconstants.WorkloadStorageKey].(string) - for storageName, cons := range c.storage { - if cons.Pool == "" && workloadStorageClass == "" { - return errors.Errorf("storage pool for %q must be specified since there's no model default storage class", storageName) - } - _, err := charmStorageParams(ctx, "", workloadStorageClass, cfg, cons.Pool, storageService, registry) - if err != nil { - return errors.Annotatef(err, "getting workload storage params for %q", c.applicationName) - } - } - return nil } diff --git a/apiserver/facades/client/application/legacy_mock_test.go b/apiserver/facades/client/application/legacy_mock_test.go index 4a393885d7b..9176d487b0e 100644 --- a/apiserver/facades/client/application/legacy_mock_test.go +++ b/apiserver/facades/client/application/legacy_mock_test.go @@ -21,7 +21,6 @@ import ( state "github.com/juju/juju/state" names "github.com/juju/names/v5" schema "github.com/juju/schema" - version "github.com/juju/version/v2" gomock "go.uber.org/mock/gomock" environschema "gopkg.in/juju/environschema.v1" ) @@ -2131,42 +2130,3 @@ func (c *MockCaasBrokerInterfaceValidateStorageClassCall) DoAndReturn(f func(con c.Call = c.Call.DoAndReturn(f) return c } - -// Version mocks base method. -func (m *MockCaasBrokerInterface) Version() (*version.Number, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Version") - ret0, _ := ret[0].(*version.Number) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// Version indicates an expected call of Version. -func (mr *MockCaasBrokerInterfaceMockRecorder) Version() *MockCaasBrokerInterfaceVersionCall { - mr.mock.ctrl.T.Helper() - call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Version", reflect.TypeOf((*MockCaasBrokerInterface)(nil).Version)) - return &MockCaasBrokerInterfaceVersionCall{Call: call} -} - -// MockCaasBrokerInterfaceVersionCall wrap *gomock.Call -type MockCaasBrokerInterfaceVersionCall struct { - *gomock.Call -} - -// Return rewrite *gomock.Call.Return -func (c *MockCaasBrokerInterfaceVersionCall) Return(arg0 *version.Number, arg1 error) *MockCaasBrokerInterfaceVersionCall { - c.Call = c.Call.Return(arg0, arg1) - return c -} - -// Do rewrite *gomock.Call.Do -func (c *MockCaasBrokerInterfaceVersionCall) Do(f func() (*version.Number, error)) *MockCaasBrokerInterfaceVersionCall { - c.Call = c.Call.Do(f) - return c -} - -// DoAndReturn rewrite *gomock.Call.DoAndReturn -func (c *MockCaasBrokerInterfaceVersionCall) DoAndReturn(f func() (*version.Number, error)) *MockCaasBrokerInterfaceVersionCall { - c.Call = c.Call.DoAndReturn(f) - return c -} diff --git a/apiserver/facades/controller/caasapplicationprovisioner/provisioner_test.go b/apiserver/facades/controller/caasapplicationprovisioner/provisioner_test.go index 6e2fb6fdb58..945b218e4e1 100644 --- a/apiserver/facades/controller/caasapplicationprovisioner/provisioner_test.go +++ b/apiserver/facades/controller/caasapplicationprovisioner/provisioner_test.go @@ -815,7 +815,6 @@ func (s *CAASApplicationProvisionerSuite) TestCharmStorageParamsPoolNotFound(c * func (s *CAASApplicationProvisionerSuite) fakeModelConfig() (*envconfig.Config, error) { attrs := coretesting.FakeConfig() - attrs["operator-storage"] = "k8s-storage" attrs["agent-version"] = "2.6-beta3.666" return envconfig.New(envconfig.UseDefaults, attrs) } diff --git a/apiserver/facades/controller/caasmodeloperator/mock_test.go b/apiserver/facades/controller/caasmodeloperator/mock_test.go index 2c1eceb3ac3..d19c88c279e 100644 --- a/apiserver/facades/controller/caasmodeloperator/mock_test.go +++ b/apiserver/facades/controller/caasmodeloperator/mock_test.go @@ -78,7 +78,6 @@ func (m *mockModel) UUID() string { func (m *mockModel) ModelConfig(_ context.Context) (*config.Config, error) { attrs := coretesting.FakeConfig() - attrs["operator-storage"] = "k8s-storage" attrs["agent-version"] = "2.6-beta3" return config.New(config.UseDefaults, attrs) } diff --git a/caas/kubernetes/metadata.go b/caas/kubernetes/metadata.go index 06aa9e57f98..b9a756d3d9b 100644 --- a/caas/kubernetes/metadata.go +++ b/caas/kubernetes/metadata.go @@ -51,7 +51,6 @@ type ClusterMetadataChecker interface { // ClusterMetadata defines metadata about a cluster. type ClusterMetadata struct { WorkloadStorageClass *storagev1.StorageClass - OperatorStorageClass *storagev1.StorageClass Cloud string Regions set.Strings } diff --git a/caas/kubernetes/provider/base_test.go b/caas/kubernetes/provider/base_test.go index 4ae2d09ad20..118ef655918 100644 --- a/caas/kubernetes/provider/base_test.go +++ b/caas/kubernetes/provider/base_test.go @@ -170,7 +170,6 @@ func (s *BaseSuite) SetUpTest(c *gc.C) { // init config for each test for easier changing config inside test. cfg, err := config.New(config.UseDefaults, coretesting.FakeConfig().Merge(coretesting.Attrs{ config.NameKey: "test", - k8sconstants.OperatorStorageKey: "", k8sconstants.WorkloadStorageKey: "", })) c.Assert(err, jc.ErrorIsNil) @@ -499,7 +498,6 @@ func (s *fakeClientSuite) SetUpTest(c *gc.C) { s.cfg, err = config.New(config.UseDefaults, coretesting.FakeConfig().Merge(coretesting.Attrs{ config.NameKey: "test", - k8sconstants.OperatorStorageKey: "", k8sconstants.WorkloadStorageKey: "", })) c.Assert(err, jc.ErrorIsNil) diff --git a/caas/kubernetes/provider/bootstrap_test.go b/caas/kubernetes/provider/bootstrap_test.go index 9821afa0792..1ddbc01bedd 100644 --- a/caas/kubernetes/provider/bootstrap_test.go +++ b/caas/kubernetes/provider/bootstrap_test.go @@ -69,7 +69,6 @@ func (s *bootstrapSuite) SetUpTest(c *gc.C) { cfg, err := config.New(config.UseDefaults, coretesting.FakeConfig().Merge(coretesting.Attrs{ config.NameKey: "controller-1", - k8sconstants.OperatorStorageKey: "", k8sconstants.WorkloadStorageKey: "", })) c.Assert(err, jc.ErrorIsNil) diff --git a/caas/kubernetes/provider/cloud.go b/caas/kubernetes/provider/cloud.go index 40244636e7f..14111742393 100644 --- a/caas/kubernetes/provider/cloud.go +++ b/caas/kubernetes/provider/cloud.go @@ -84,19 +84,10 @@ func UpdateKubeCloudWithStorage(k8sCloud cloud.Cloud, storageParams KubeCloudSto } } - // We at least expected operator storage to be available to successfully use - // this cloud. - if clusterMetadata.OperatorStorageClass == nil { - return cloud.Cloud{}, &environs.PreferredStorageNotFound{ - Message: "no preferred operator storage found in Kubernetes cluster", - } - } - if k8sCloud.Config == nil { k8sCloud.Config = make(map[string]interface{}) } - k8sCloud.Config[k8sconstants.OperatorStorageKey] = clusterMetadata.OperatorStorageClass.Name k8sCloud.Config[k8sconstants.WorkloadStorageKey] = "" if clusterMetadata.WorkloadStorageClass != nil { k8sCloud.Config[k8sconstants.WorkloadStorageKey] = clusterMetadata.WorkloadStorageClass.Name @@ -140,7 +131,7 @@ func (p kubernetesEnvironProvider) FinalizeCloud(ctx environs.FinalizeCloudConte cld.AuthTypes = k8scloud.SupportedAuthTypes() // if storage is already defined there is no need to query the cluster - if opStorage, ok := cld.Config[k8sconstants.OperatorStorageKey]; ok && opStorage != "" { + if opStorage, ok := cld.Config[k8sconstants.WorkloadStorageKey]; ok && opStorage != "" { return cld, nil } diff --git a/caas/kubernetes/provider/cloud_test.go b/caas/kubernetes/provider/cloud_test.go index e1aa6554176..05ce9498a6d 100644 --- a/caas/kubernetes/provider/cloud_test.go +++ b/caas/kubernetes/provider/cloud_test.go @@ -19,7 +19,6 @@ import ( "github.com/juju/juju/caas" k8s "github.com/juju/juju/caas/kubernetes" - k8scloud "github.com/juju/juju/caas/kubernetes/cloud" "github.com/juju/juju/caas/kubernetes/provider" k8sutils "github.com/juju/juju/caas/kubernetes/provider/utils" jujucloud "github.com/juju/juju/cloud" @@ -45,9 +44,9 @@ var defaultK8sCloud = jujucloud.Cloud{ var defaultClusterMetadata = &k8s.ClusterMetadata{ Cloud: k8s.K8sCloudMicrok8s, Regions: set.NewStrings(k8s.Microk8sRegion), - OperatorStorageClass: &storagev1.StorageClass{ + WorkloadStorageClass: &storagev1.StorageClass{ ObjectMeta: metav1.ObjectMeta{ - Name: "operator-sc", + Name: "workload-sc", }, }, } @@ -104,37 +103,7 @@ func (s *cloudSuite) TestFinalizeCloudMicrok8s(c *gc.C) { SkipTLSVerify: true, Endpoint: "http://1.1.1.1:8080", HostCloudRegion: fmt.Sprintf("%s/%s", k8s.K8sCloudMicrok8s, k8s.Microk8sRegion), - Config: map[string]interface{}{"operator-storage": "operator-sc", "workload-storage": ""}, - Regions: []jujucloud.Region{{Name: k8s.Microk8sRegion, Endpoint: "http://1.1.1.1:8080"}}, - }) -} - -func (s *cloudSuite) TestFinalizeCloudMicrok8sAlreadyStorage(c *gc.C) { - preparedCloud := jujucloud.Cloud{ - Name: k8s.K8sCloudMicrok8s, - Type: jujucloud.CloudTypeKubernetes, - AuthTypes: []jujucloud.AuthType{jujucloud.UserPassAuthType}, - CACertificates: []string{""}, - Endpoint: "http://1.1.1.1:8080", - HostCloudRegion: fmt.Sprintf("%s/%s", k8s.K8sCloudMicrok8s, k8s.Microk8sRegion), - Config: map[string]interface{}{"operator-storage": "something-else", "workload-storage": ""}, - Regions: []jujucloud.Region{{Name: k8s.Microk8sRegion, Endpoint: "http://1.1.1.1:8080"}}, - } - - p := s.getProvider() - cloudFinalizer := p.(environs.CloudFinalizer) - - ctx := mockContext{Context: context.Background()} - cloud, err := cloudFinalizer.FinalizeCloud(&ctx, preparedCloud) - c.Assert(err, jc.ErrorIsNil) - c.Assert(cloud, jc.DeepEquals, jujucloud.Cloud{ - Name: k8s.K8sCloudMicrok8s, - Type: jujucloud.CloudTypeKubernetes, - AuthTypes: k8scloud.SupportedAuthTypes(), - CACertificates: []string{""}, - Endpoint: "http://1.1.1.1:8080", - HostCloudRegion: fmt.Sprintf("%s/%s", k8s.K8sCloudMicrok8s, k8s.Microk8sRegion), - Config: map[string]interface{}{"operator-storage": "something-else", "workload-storage": ""}, + Config: map[string]interface{}{"workload-storage": "workload-sc"}, Regions: []jujucloud.Region{{Name: k8s.Microk8sRegion, Endpoint: "http://1.1.1.1:8080"}}, }) } diff --git a/caas/kubernetes/provider/constants/storage.go b/caas/kubernetes/provider/constants/storage.go index a82fc152c55..3006cbe4d05 100644 --- a/caas/kubernetes/provider/constants/storage.go +++ b/caas/kubernetes/provider/constants/storage.go @@ -28,10 +28,6 @@ const ( // WorkloadStorageKey is the model config attribute used to specify // the storage class for provisioning workload storage. WorkloadStorageKey = "workload-storage" - - // OperatorStorageKey is the model config attribute used to specify - // the storage class for provisioning operator storage. - OperatorStorageKey = "operator-storage" ) // QualifiedStorageClassName returns a qualified storage class name. diff --git a/caas/kubernetes/provider/k8s.go b/caas/kubernetes/provider/k8s.go index b54f22610c8..1f0289ed6a0 100644 --- a/caas/kubernetes/provider/k8s.go +++ b/caas/kubernetes/provider/k8s.go @@ -387,7 +387,7 @@ Please bootstrap again and choose a different controller name.`, controllerName) // The namespace will be set to controller-name in newcontrollerStack. // do validation on storage class. - _, err = k.validateOperatorStorage(ctx) + _, err = k.validateControllerWorkloadStorage(ctx) return errors.Trace(err) } @@ -417,10 +417,10 @@ func (k *kubernetesClient) EnsureImageRepoSecret(ctx context.Context, imageRepo return errors.Trace(err) } -func (k *kubernetesClient) validateOperatorStorage(ctx context.Context) (string, error) { - storageClass, _ := k.Config().AllAttrs()[constants.OperatorStorageKey].(string) +func (k *kubernetesClient) validateControllerWorkloadStorage(ctx context.Context) (string, error) { + storageClass, _ := k.Config().AllAttrs()[constants.WorkloadStorageKey].(string) if storageClass == "" { - return "", errors.NewNotValid(nil, "config without operator-storage value not valid.\nRun juju add-k8s to reimport your k8s cluster.") + return "", errors.NewNotValid(nil, "config without workload-storage value not valid.\nRun juju add-k8s to reimport your k8s cluster.") } _, err := k.getStorageClass(ctx, storageClass) return storageClass, errors.Trace(err) @@ -437,7 +437,8 @@ func (k *kubernetesClient) Bootstrap( return nil, errors.NotSupportedf("set base for bootstrapping to kubernetes") } - storageClass, err := k.validateOperatorStorage(ctx) + // Validate workload storage is available to the controller. + storageClass, err := k.validateControllerWorkloadStorage(ctx) if err != nil { return nil, errors.Trace(err) } diff --git a/caas/kubernetes/provider/k8s_test.go b/caas/kubernetes/provider/k8s_test.go index c43726fdea1..6af0ea1e204 100644 --- a/caas/kubernetes/provider/k8s_test.go +++ b/caas/kubernetes/provider/k8s_test.go @@ -245,7 +245,7 @@ func (s *K8sBrokerSuite) TestSetConfig(c *gc.C) { c.Assert(err, jc.ErrorIsNil) } -func (s *K8sBrokerSuite) TestBootstrapNoOperatorStorage(c *gc.C) { +func (s *K8sBrokerSuite) TestBootstrapNoWorkloadStorage(c *gc.C) { ctrl := s.setupController(c) defer ctrl.Finish() @@ -260,15 +260,15 @@ func (s *K8sBrokerSuite) TestBootstrapNoOperatorStorage(c *gc.C) { _, err := s.broker.Bootstrap(ctx, callCtx, bootstrapParams) c.Assert(err, gc.NotNil) msg := strings.Replace(err.Error(), "\n", "", -1) - c.Assert(msg, gc.Matches, "config without operator-storage value not valid.*") + c.Assert(msg, gc.Matches, "config without workload-storage value not valid.*") } func (s *K8sBrokerSuite) TestBootstrap(c *gc.C) { ctrl := s.setupController(c) defer ctrl.Finish() - // Ensure the broker is configured with operator storage. - s.setupOperatorStorageConfig(c) + // Ensure the broker is configured with workload storage. + s.setupWorkloadStorageConfig(c) ctx := envtesting.BootstrapContext(context.Background(), c) callCtx := envcontext.WithoutCredentialInvalidator(ctx) @@ -284,10 +284,7 @@ func (s *K8sBrokerSuite) TestBootstrap(c *gc.C) { }, } gomock.InOrder( - // Check the operator storage exists. s.mockStorageClass.EXPECT().Get(gomock.Any(), "test-some-storage", v1.GetOptions{}). - Return(nil, s.k8sNotFoundError()), - s.mockStorageClass.EXPECT().Get(gomock.Any(), "some-storage", v1.GetOptions{}). Return(sc, nil), ) result, err := s.broker.Bootstrap(ctx, callCtx, bootstrapParams) @@ -300,10 +297,10 @@ func (s *K8sBrokerSuite) TestBootstrap(c *gc.C) { c.Assert(err, jc.ErrorIs, errors.NotSupported) } -func (s *K8sBrokerSuite) setupOperatorStorageConfig(c *gc.C) { +func (s *K8sBrokerSuite) setupWorkloadStorageConfig(c *gc.C) { cfg := s.broker.Config() var err error - cfg, err = cfg.Apply(map[string]interface{}{"operator-storage": "some-storage"}) + cfg, err = cfg.Apply(map[string]interface{}{"workload-storage": "some-storage"}) c.Assert(err, jc.ErrorIsNil) err = s.broker.SetConfig(context.Background(), cfg) c.Assert(err, jc.ErrorIsNil) @@ -313,8 +310,8 @@ func (s *K8sBrokerSuite) TestPrepareForBootstrap(c *gc.C) { ctrl := s.setupController(c) defer ctrl.Finish() - // Ensure the broker is configured with operator storage. - s.setupOperatorStorageConfig(c) + // Ensure the broker is configured with workload storage. + s.setupWorkloadStorageConfig(c) sc := &storagev1.StorageClass{ ObjectMeta: v1.ObjectMeta{ diff --git a/caas/kubernetes/provider/metadata.go b/caas/kubernetes/provider/metadata.go index 37a6dc9cdc0..beb47a19b8b 100644 --- a/caas/kubernetes/provider/metadata.go +++ b/caas/kubernetes/provider/metadata.go @@ -134,18 +134,11 @@ func GetClusterMetadata( return nil, errors.Annotate(err, "cannot determine cluster region") } - // We may have the workload storage but still need to look for operator storage. storageClasses, err := storageClassI.List(ctx, v1.ListOptions{}) if err != nil { return nil, errors.Annotate(err, "listing storage classes") } - preferredOperatorStorage := providerstorage.PreferredOperatorStorageForCloud(result.Cloud).Prepend( - &providerstorage.PreferredStorageNominated{ - StorageClassName: nominatedStorageClass, - }, - ) - preferredWorkloadStorage := providerstorage.PreferredWorkloadStorageForCloud(result.Cloud).Prepend( &providerstorage.PreferredStorageNominated{ StorageClassName: nominatedStorageClass, @@ -153,19 +146,11 @@ func GetClusterMetadata( ) var ( - selectedOperatorSC *storagev1.StorageClass selectedWorkloadSC *storagev1.StorageClass - operatorPriority int workloadPriority int ) for i, sc := range storageClasses.Items { - priority, matches := preferredOperatorStorage.Matches(&sc) - if matches && (priority < operatorPriority || selectedOperatorSC == nil) { - selectedOperatorSC = &storageClasses.Items[i] - operatorPriority = priority - } - - priority, matches = preferredWorkloadStorage.Matches(&sc) + priority, matches := preferredWorkloadStorage.Matches(&sc) if matches && (priority < workloadPriority || selectedWorkloadSC == nil) { selectedWorkloadSC = &storageClasses.Items[i] workloadPriority = priority @@ -173,12 +158,6 @@ func GetClusterMetadata( } if nominatedStorageClass != "" { - if selectedOperatorSC == nil || selectedOperatorSC.Name != nominatedStorageClass { - return nil, &environs.NominatedStorageNotFound{ - StorageName: nominatedStorageClass, - } - } - if selectedWorkloadSC == nil || selectedWorkloadSC.Name != nominatedStorageClass { return nil, &environs.NominatedStorageNotFound{ StorageName: nominatedStorageClass, @@ -186,7 +165,6 @@ func GetClusterMetadata( } } - result.OperatorStorageClass = selectedOperatorSC result.WorkloadStorageClass = selectedWorkloadSC return &result, nil } diff --git a/caas/kubernetes/provider/metadata_test.go b/caas/kubernetes/provider/metadata_test.go index 8081939e14d..2e73d14de78 100644 --- a/caas/kubernetes/provider/metadata_test.go +++ b/caas/kubernetes/provider/metadata_test.go @@ -28,15 +28,6 @@ type K8sMetadataSuite struct { var _ = gc.Suite(&K8sMetadataSuite{}) var ( - annotatedOperatorStorage = &storagev1.StorageClass{ - ObjectMeta: v1.ObjectMeta{ - Name: "operator-storage", - Annotations: map[string]string{ - "juju.is/operator-storage": "true", - }, - }, - } - annotatedWorkloadStorage = &storagev1.StorageClass{ ObjectMeta: v1.ObjectMeta{ Name: "workload-storage", @@ -304,7 +295,6 @@ func (_ *K8sMetadataSuite) TestGetMetadataVariations(c *gc.C) { Cloud: "ec2", Regions: set.NewStrings("wallyworld-region"), WorkloadStorageClass: ec2StorageClass, - OperatorStorageClass: ec2StorageClass, }, }, { @@ -312,27 +302,23 @@ func (_ *K8sMetadataSuite) TestGetMetadataVariations(c *gc.C) { InitialObjects: []runtime.Object{ ec2Node, ec2StorageClass, - annotatedOperatorStorage, annotatedWorkloadStorage, }, Result: kubernetes.ClusterMetadata{ Cloud: "ec2", Regions: set.NewStrings("wallyworld-region"), WorkloadStorageClass: annotatedWorkloadStorage, - OperatorStorageClass: annotatedOperatorStorage, }, }, { Name: "Test ec2 cloud prefers annotation storage without workload", InitialObjects: []runtime.Object{ ec2Node, - annotatedOperatorStorage, }, Result: kubernetes.ClusterMetadata{ Cloud: "ec2", Regions: set.NewStrings("wallyworld-region"), WorkloadStorageClass: nil, - OperatorStorageClass: annotatedOperatorStorage, }, }, { @@ -340,7 +326,6 @@ func (_ *K8sMetadataSuite) TestGetMetadataVariations(c *gc.C) { InitialObjects: []runtime.Object{ ec2Node, ec2StorageClass, - annotatedOperatorStorage, annotatedWorkloadStorage, nominatedStorage, }, @@ -349,7 +334,6 @@ func (_ *K8sMetadataSuite) TestGetMetadataVariations(c *gc.C) { Cloud: "ec2", Regions: set.NewStrings("wallyworld-region"), WorkloadStorageClass: nominatedStorage, - OperatorStorageClass: nominatedStorage, }, }, { @@ -361,7 +345,6 @@ func (_ *K8sMetadataSuite) TestGetMetadataVariations(c *gc.C) { Cloud: "ec2", Regions: set.NewStrings("wallyworld-region"), WorkloadStorageClass: nil, - OperatorStorageClass: nil, }, }, { @@ -374,7 +357,6 @@ func (_ *K8sMetadataSuite) TestGetMetadataVariations(c *gc.C) { Cloud: "ec2", Regions: set.NewStrings("wallyworld-region"), WorkloadStorageClass: defaultStorage, - OperatorStorageClass: defaultStorage, }, }, @@ -389,7 +371,6 @@ func (_ *K8sMetadataSuite) TestGetMetadataVariations(c *gc.C) { Cloud: "microk8s", Regions: set.NewStrings("wallyworld-region"), WorkloadStorageClass: microk8sStorageClass, - OperatorStorageClass: microk8sStorageClass, }, }, { @@ -397,27 +378,23 @@ func (_ *K8sMetadataSuite) TestGetMetadataVariations(c *gc.C) { InitialObjects: []runtime.Object{ microk8sNode, microk8sStorageClass, - annotatedOperatorStorage, annotatedWorkloadStorage, }, Result: kubernetes.ClusterMetadata{ Cloud: "microk8s", Regions: set.NewStrings("wallyworld-region"), WorkloadStorageClass: annotatedWorkloadStorage, - OperatorStorageClass: annotatedOperatorStorage, }, }, { Name: "Test microk8s cloud prefers annotation storage without workload", InitialObjects: []runtime.Object{ microk8sNode, - annotatedOperatorStorage, }, Result: kubernetes.ClusterMetadata{ Cloud: "microk8s", Regions: set.NewStrings("wallyworld-region"), WorkloadStorageClass: nil, - OperatorStorageClass: annotatedOperatorStorage, }, }, { @@ -425,7 +402,6 @@ func (_ *K8sMetadataSuite) TestGetMetadataVariations(c *gc.C) { InitialObjects: []runtime.Object{ microk8sNode, microk8sStorageClass, - annotatedOperatorStorage, annotatedWorkloadStorage, nominatedStorage, }, @@ -434,7 +410,6 @@ func (_ *K8sMetadataSuite) TestGetMetadataVariations(c *gc.C) { Cloud: "microk8s", Regions: set.NewStrings("wallyworld-region"), WorkloadStorageClass: nominatedStorage, - OperatorStorageClass: nominatedStorage, }, }, { @@ -446,7 +421,6 @@ func (_ *K8sMetadataSuite) TestGetMetadataVariations(c *gc.C) { Cloud: "microk8s", Regions: set.NewStrings("wallyworld-region"), WorkloadStorageClass: nil, - OperatorStorageClass: nil, }, }, { @@ -459,7 +433,6 @@ func (_ *K8sMetadataSuite) TestGetMetadataVariations(c *gc.C) { Cloud: "microk8s", Regions: set.NewStrings("wallyworld-region"), WorkloadStorageClass: nil, - OperatorStorageClass: nil, }, }, @@ -474,7 +447,6 @@ func (_ *K8sMetadataSuite) TestGetMetadataVariations(c *gc.C) { Cloud: "azure", Regions: set.NewStrings("wallyworld-region"), WorkloadStorageClass: azureStorageClass, - OperatorStorageClass: azureStorageClass, }, }, { @@ -482,27 +454,23 @@ func (_ *K8sMetadataSuite) TestGetMetadataVariations(c *gc.C) { InitialObjects: []runtime.Object{ azureNode, azureStorageClass, - annotatedOperatorStorage, annotatedWorkloadStorage, }, Result: kubernetes.ClusterMetadata{ Cloud: "azure", Regions: set.NewStrings("wallyworld-region"), WorkloadStorageClass: annotatedWorkloadStorage, - OperatorStorageClass: annotatedOperatorStorage, }, }, { Name: "Test azure cloud prefers annotation storage without workload", InitialObjects: []runtime.Object{ azureNode, - annotatedOperatorStorage, }, Result: kubernetes.ClusterMetadata{ Cloud: "azure", Regions: set.NewStrings("wallyworld-region"), WorkloadStorageClass: nil, - OperatorStorageClass: annotatedOperatorStorage, }, }, { @@ -510,7 +478,6 @@ func (_ *K8sMetadataSuite) TestGetMetadataVariations(c *gc.C) { InitialObjects: []runtime.Object{ azureNode, azureStorageClass, - annotatedOperatorStorage, annotatedWorkloadStorage, nominatedStorage, }, @@ -519,7 +486,6 @@ func (_ *K8sMetadataSuite) TestGetMetadataVariations(c *gc.C) { Cloud: "azure", Regions: set.NewStrings("wallyworld-region"), WorkloadStorageClass: nominatedStorage, - OperatorStorageClass: nominatedStorage, }, }, { @@ -531,7 +497,6 @@ func (_ *K8sMetadataSuite) TestGetMetadataVariations(c *gc.C) { Cloud: "azure", Regions: set.NewStrings("wallyworld-region"), WorkloadStorageClass: nil, - OperatorStorageClass: nil, }, }, { @@ -544,7 +509,6 @@ func (_ *K8sMetadataSuite) TestGetMetadataVariations(c *gc.C) { Cloud: "azure", Regions: set.NewStrings("wallyworld-region"), WorkloadStorageClass: defaultStorage, - OperatorStorageClass: defaultStorage, }, }, @@ -559,7 +523,6 @@ func (_ *K8sMetadataSuite) TestGetMetadataVariations(c *gc.C) { Cloud: "gce", Regions: set.NewStrings("wallyworld-region"), WorkloadStorageClass: gceStorageClass, - OperatorStorageClass: gceStorageClass, }, }, { @@ -567,27 +530,23 @@ func (_ *K8sMetadataSuite) TestGetMetadataVariations(c *gc.C) { InitialObjects: []runtime.Object{ gceNode, gceStorageClass, - annotatedOperatorStorage, annotatedWorkloadStorage, }, Result: kubernetes.ClusterMetadata{ Cloud: "gce", Regions: set.NewStrings("wallyworld-region"), WorkloadStorageClass: annotatedWorkloadStorage, - OperatorStorageClass: annotatedOperatorStorage, }, }, { Name: "Test gce cloud prefers annotation storage without workload", InitialObjects: []runtime.Object{ gceNode, - annotatedOperatorStorage, }, Result: kubernetes.ClusterMetadata{ Cloud: "gce", Regions: set.NewStrings("wallyworld-region"), WorkloadStorageClass: nil, - OperatorStorageClass: annotatedOperatorStorage, }, }, { @@ -595,7 +554,6 @@ func (_ *K8sMetadataSuite) TestGetMetadataVariations(c *gc.C) { InitialObjects: []runtime.Object{ gceNode, gceStorageClass, - annotatedOperatorStorage, annotatedWorkloadStorage, nominatedStorage, }, @@ -604,7 +562,6 @@ func (_ *K8sMetadataSuite) TestGetMetadataVariations(c *gc.C) { Cloud: "gce", Regions: set.NewStrings("wallyworld-region"), WorkloadStorageClass: nominatedStorage, - OperatorStorageClass: nominatedStorage, }, }, { @@ -616,7 +573,6 @@ func (_ *K8sMetadataSuite) TestGetMetadataVariations(c *gc.C) { Cloud: "gce", Regions: set.NewStrings("wallyworld-region"), WorkloadStorageClass: nil, - OperatorStorageClass: nil, }, }, { @@ -629,7 +585,6 @@ func (_ *K8sMetadataSuite) TestGetMetadataVariations(c *gc.C) { Cloud: "gce", Regions: set.NewStrings("wallyworld-region"), WorkloadStorageClass: defaultStorage, - OperatorStorageClass: defaultStorage, }, }, @@ -639,27 +594,12 @@ func (_ *K8sMetadataSuite) TestGetMetadataVariations(c *gc.C) { InitialObjects: []runtime.Object{ newNode(map[string]string{}), gceStorageClass, - annotatedOperatorStorage, annotatedWorkloadStorage, }, Result: kubernetes.ClusterMetadata{ Cloud: "", Regions: set.NewStrings(), WorkloadStorageClass: annotatedWorkloadStorage, - OperatorStorageClass: annotatedOperatorStorage, - }, - }, - { - Name: "Test other cloud prefers annotation storage without workload", - InitialObjects: []runtime.Object{ - newNode(map[string]string{}), - annotatedOperatorStorage, - }, - Result: kubernetes.ClusterMetadata{ - Cloud: "", - Regions: set.NewStrings(), - WorkloadStorageClass: annotatedOperatorStorage, - OperatorStorageClass: annotatedOperatorStorage, }, }, { @@ -667,7 +607,6 @@ func (_ *K8sMetadataSuite) TestGetMetadataVariations(c *gc.C) { InitialObjects: []runtime.Object{ newNode(map[string]string{}), gceStorageClass, - annotatedOperatorStorage, annotatedWorkloadStorage, nominatedStorage, }, @@ -676,7 +615,6 @@ func (_ *K8sMetadataSuite) TestGetMetadataVariations(c *gc.C) { Cloud: "", Regions: set.NewStrings(), WorkloadStorageClass: nominatedStorage, - OperatorStorageClass: nominatedStorage, }, }, { @@ -688,7 +626,6 @@ func (_ *K8sMetadataSuite) TestGetMetadataVariations(c *gc.C) { Cloud: "", Regions: set.NewStrings(), WorkloadStorageClass: nil, - OperatorStorageClass: nil, }, }, { @@ -701,7 +638,6 @@ func (_ *K8sMetadataSuite) TestGetMetadataVariations(c *gc.C) { Cloud: "", Regions: set.NewStrings(), WorkloadStorageClass: defaultStorage, - OperatorStorageClass: defaultStorage, }, }, } @@ -725,7 +661,6 @@ func (s *K8sMetadataSuite) TestNominatedStorageNotFound(c *gc.C) { clientSet := fake.NewSimpleClientset( newNode(map[string]string{}), gceStorageClass, - annotatedOperatorStorage, annotatedWorkloadStorage, ) diff --git a/caas/kubernetes/provider/provider_test.go b/caas/kubernetes/provider/provider_test.go index 796638f9e68..a4169e55eec 100644 --- a/caas/kubernetes/provider/provider_test.go +++ b/caas/kubernetes/provider/provider_test.go @@ -31,7 +31,6 @@ func fakeConfigAttrs(attrs ...coretesting.Attrs) coretesting.Attrs { merged := coretesting.FakeConfig().Merge(coretesting.Attrs{ "type": "kubernetes", "uuid": uuid.MustNewUUID().String(), - "operator-storage": "", "workload-storage": "", }) for _, attrs := range attrs { diff --git a/caas/kubernetes/provider/providerconfig.go b/caas/kubernetes/provider/providerconfig.go index e2ef1547d3c..8a58154603c 100644 --- a/caas/kubernetes/provider/providerconfig.go +++ b/caas/kubernetes/provider/providerconfig.go @@ -8,43 +8,19 @@ import ( "fmt" "github.com/juju/schema" - "github.com/juju/version/v2" "gopkg.in/juju/environschema.v1" "github.com/juju/juju/caas/kubernetes/provider/constants" k8sconstants "github.com/juju/juju/caas/kubernetes/provider/constants" "github.com/juju/juju/environs/config" - "github.com/juju/juju/internal/charm" ) -var ( - // jujuVersionForControllerStorage is the Juju version which first - // added the ability to store charm state in the controller. - jujuVersionForControllerStorage = version.MustParse("2.8.0") -) - -// RequireOperatorStorage returns true if the specified min-juju-version -// defined by a charm is such that the charm requires operator storage. -func RequireOperatorStorage(ch charm.CharmMeta) bool { - if charm.MetaFormat(ch) == charm.FormatV2 { - return false - } - minVers := ch.Meta().MinJujuVersion - return minVers.Compare(jujuVersionForControllerStorage) < 0 -} - var configSchema = environschema.Fields{ k8sconstants.WorkloadStorageKey: { Description: "The preferred storage class used to provision workload storage.", Type: environschema.Tstring, Group: environschema.AccountGroup, }, - k8sconstants.OperatorStorageKey: { - Description: "The storage class used to provision operator storage.", - Type: environschema.Tstring, - Group: environschema.AccountGroup, - Immutable: true, - }, } var providerConfigFields = func() schema.Fields { @@ -57,7 +33,6 @@ var providerConfigFields = func() schema.Fields { var providerConfigDefaults = schema.Defaults{ k8sconstants.WorkloadStorageKey: "", - k8sconstants.OperatorStorageKey: "", } type brokerConfig struct { diff --git a/caas/kubernetes/provider/rbac_test.go b/caas/kubernetes/provider/rbac_test.go index 532b44f21e1..d50fe485dff 100644 --- a/caas/kubernetes/provider/rbac_test.go +++ b/caas/kubernetes/provider/rbac_test.go @@ -103,7 +103,6 @@ func (s *rbacSuite) TestEnsureSecretAccessTokenCreate(c *gc.C) { func (s *rbacSuite) switchToControllerModel(c *gc.C) { cfg, err := config.New(config.UseDefaults, coretesting.FakeConfig().Merge(coretesting.Attrs{ config.NameKey: environsbootstrap.ControllerModelName, - k8sconstants.OperatorStorageKey: "", k8sconstants.WorkloadStorageKey: "", })) c.Assert(err, jc.ErrorIsNil) diff --git a/caas/kubernetes/provider/storage/metadata.go b/caas/kubernetes/provider/storage/metadata.go index ddfdfd3198f..1ad5702e568 100644 --- a/caas/kubernetes/provider/storage/metadata.go +++ b/caas/kubernetes/provider/storage/metadata.go @@ -33,11 +33,7 @@ type PreferredStorage interface { Matches(*storagev1.StorageClass) bool } -// PreferredStorageOperatorAnnotation is an implementation of PreferredStorage -// that matches based on the operator storage annotation. -type PreferredStorageOperatorAnnotation struct{} - -// PreferredStorageOperatorAnnotation is an implementation of PreferredStorage +// PreferredStorageWorkloadAnnotation is an implementation of PreferredStorage // that matches based on the workload storage annotation. type PreferredStorageWorkloadAnnotation struct{} @@ -64,79 +60,9 @@ type PreferredStorageProvisioner struct { type PreferredStorageList []PreferredStorage const ( - operatorStorageClassAnnotationKey = "juju.is/operator-storage" workloadStorageClassAnnotationKey = "juju.is/workload-storage" ) -// PreferredStorageForCloud returns a PreferredStorageList for the supplied -// cloud. If no cloud is found matching then a default list is provided. -func PreferredOperatorStorageForCloud(cloud string) PreferredStorageList { - switch cloud { - - // Microk8s - case kubernetes.K8sCloudMicrok8s: - return PreferredStorageList{ - &PreferredStorageOperatorAnnotation{}, - &PreferredStorageProvisioner{ - NameVal: "hostpath", - Provisioner: "microk8s.io/hostpath", - }, - } - - // Azure - case kubernetes.K8sCloudAzure: - return PreferredStorageList{ - &PreferredStorageOperatorAnnotation{}, - &PreferredStorageProvisioner{ - NameVal: "azure-disk", - Provisioner: "kubernetes.io/azure-disk", - }, - &PreferredStorageDefault{}, - } - - // Google Cloud - case kubernetes.K8sCloudGCE: - return PreferredStorageList{ - &PreferredStorageOperatorAnnotation{}, - &PreferredStorageProvisioner{ - NameVal: "gce-pd", - Provisioner: "kubernetes.io/gce-pd", - }, - &PreferredStorageDefault{}, - } - - // AWS - case kubernetes.K8sCloudEC2: - return PreferredStorageList{ - &PreferredStorageOperatorAnnotation{}, - &PreferredStorageProvisioner{ - NameVal: "aws-ebs", - Provisioner: "kubernetes.io/aws-ebs", - }, - &PreferredStorageDefault{}, - } - - // Openstack - case kubernetes.K8sCloudOpenStack: - return PreferredStorageList{ - &PreferredStorageOperatorAnnotation{}, - &PreferredStorageProvisioner{ - NameVal: "csi-cinder", - Provisioner: "csi-cinderplugin", - }, - &PreferredStorageDefault{}, - } - - // Everything else - default: - return PreferredStorageList{ - &PreferredStorageOperatorAnnotation{}, - &PreferredStorageDefault{}, - &PreferredStorageAny{}, - } - } -} - // PreferredWorkloadStorageForCloud returns a PreferredStorageList for the // supplied cloud. If no cloud is found matching then a default list is // provided. @@ -243,13 +169,6 @@ func (p *PreferredStorageNominated) Matches(sc *storagev1.StorageClass) bool { return sc.Name == p.StorageClassName } -// Matches implements PreferredStorage Matches. -func (_ *PreferredStorageOperatorAnnotation) Matches(sc *storagev1.StorageClass) bool { - return k8sannotations.New(sc.GetAnnotations()).Has( - operatorStorageClassAnnotationKey, "true", - ) -} - // Matches implements PreferredStorage Matches. func (_ *PreferredStorageWorkloadAnnotation) Matches(sc *storagev1.StorageClass) bool { return k8sannotations.New(sc.GetAnnotations()).Has( @@ -283,11 +202,6 @@ func (_ *PreferredStorageNominated) Name() string { return "nominated-storage" } -// Name implements PreferredStorage Name. -func (_ *PreferredStorageOperatorAnnotation) Name() string { - return "operator-storage-annotation" -} - // Name implements PreferredStorage Name. func (_ *PreferredStorageWorkloadAnnotation) Name() string { return "workload-storage-annotation" diff --git a/caas/kubernetes/provider/storage/metadata_test.go b/caas/kubernetes/provider/storage/metadata_test.go index b72f8d45c2f..7e7574b92fa 100644 --- a/caas/kubernetes/provider/storage/metadata_test.go +++ b/caas/kubernetes/provider/storage/metadata_test.go @@ -96,57 +96,6 @@ func (*metadataSuite) TestPreferredStorageNominated(c *gc.C) { } } -func (*metadataSuite) TestPreferredStorageOperatorAnnotation(c *gc.C) { - tests := []struct { - Name string - StorageClass *storagev1.StorageClass - Result bool - }{ - { - Name: "Test operator storage annotation matches", - StorageClass: &storagev1.StorageClass{ - ObjectMeta: meta.ObjectMeta{ - Name: "test1", - Annotations: map[string]string{ - "juju.is/operator-storage": "true", - }, - }, - }, - Result: true, - }, - { - Name: "Test operator storage doesn't match bad value", - StorageClass: &storagev1.StorageClass{ - ObjectMeta: meta.ObjectMeta{ - Name: "test1", - Annotations: map[string]string{ - "juju.is/operator-storage": "false", - }, - }, - }, - Result: false, - }, - { - Name: "Test operator storage doesn't match workload storage", - StorageClass: &storagev1.StorageClass{ - ObjectMeta: meta.ObjectMeta{ - Name: "test1", - Annotations: map[string]string{ - "juju.is/workload-storage": "true", - }, - }, - }, - Result: false, - }, - } - - for _, test := range tests { - c.Logf("running test %s", test.Name) - annotation := storage.PreferredStorageOperatorAnnotation{} - c.Assert(annotation.Matches(test.StorageClass), gc.Equals, test.Result) - } -} - func (*metadataSuite) TestPreferredStorageWorkloadAnnotation(c *gc.C) { tests := []struct { Name string @@ -154,7 +103,7 @@ func (*metadataSuite) TestPreferredStorageWorkloadAnnotation(c *gc.C) { Result bool }{ { - Name: "Test operator storage annotation matches", + Name: "Test workload storage annotation matches", StorageClass: &storagev1.StorageClass{ ObjectMeta: meta.ObjectMeta{ Name: "test1", @@ -166,7 +115,7 @@ func (*metadataSuite) TestPreferredStorageWorkloadAnnotation(c *gc.C) { Result: true, }, { - Name: "Test operator storage doesn't match bad value", + Name: "Test workload storage doesn't match bad value", StorageClass: &storagev1.StorageClass{ ObjectMeta: meta.ObjectMeta{ Name: "test1", @@ -177,18 +126,6 @@ func (*metadataSuite) TestPreferredStorageWorkloadAnnotation(c *gc.C) { }, Result: false, }, - { - Name: "Test operator storage doesn't match operator storage", - StorageClass: &storagev1.StorageClass{ - ObjectMeta: meta.ObjectMeta{ - Name: "test1", - Annotations: map[string]string{ - "juju.is/operator-storage": "true", - }, - }, - }, - Result: false, - }, } for _, test := range tests { diff --git a/cmd/juju/application/deploy_test.go b/cmd/juju/application/deploy_test.go index 21dd5f6a4a6..3f8008f2a4d 100644 --- a/cmd/juju/application/deploy_test.go +++ b/cmd/juju/application/deploy_test.go @@ -473,7 +473,7 @@ func (s *CAASDeploySuiteBase) fakeAPI() *fakeDeployAPI { "name": "sword", "uuid": coretesting.ModelTag.Id(), "type": model.CAAS, - "operator-storage": "k8s-storage", + "workload-storage": "k8s-storage", "secret-backend": "auto", } fakeAPI := vanillaFakeModelAPI(cfgAttrs) diff --git a/cmd/juju/caas/add.go b/cmd/juju/caas/add.go index 8e9b495438d..2d81f3ceea1 100644 --- a/cmd/juju/caas/add.go +++ b/cmd/juju/caas/add.go @@ -93,10 +93,10 @@ or Region is strictly necessary only when adding a k8s cluster to a JAAS controller. When using a standalone Juju controller, usually just --cloud is required. -Once Juju is aware of the underlying cloud type, it looks for a suitably configured -storage class to provide operator and workload storage. If none is found, use -of the --storage option is required so that Juju will create a storage class -with the specified name. +Once Juju is aware of the underlying cloud type, it looks for a suitably +configured storage class to provide workload storage. If none is found, use of +the --storage option is required so that Juju will select (or create if not +already present) a storage class with the specified name. If the cluster does not have a storage provisioning capability, use the --skip-storage option to add the cluster without any workload storage configured. diff --git a/cmd/juju/caas/add_test.go b/cmd/juju/caas/add_test.go index 3201e95c271..ff6ad34e1f2 100644 --- a/cmd/juju/caas/add_test.go +++ b/cmd/juju/caas/add_test.go @@ -313,8 +313,8 @@ func (s *addCAASSuite) SetUpTest(c *gc.C) { defaultClusterMetadata := &k8s.ClusterMetadata{ Cloud: "gce", Regions: set.NewStrings("us-east1"), - OperatorStorageClass: &storagev1.StorageClass{ - ObjectMeta: meta.ObjectMeta{Name: "operator-sc"}, + WorkloadStorageClass: &storagev1.StorageClass{ + ObjectMeta: meta.ObjectMeta{Name: "workload-sc"}, }, } s.fakeK8sClusterMetadataChecker = &fakeK8sClusterMetadataChecker{ @@ -775,7 +775,7 @@ func (s *addCAASSuite) TestGatherClusterRegionMetaRegionNoMatchesThenIgnored(c * IdentityEndpoint: "", StorageEndpoint: "", Regions: []cloud.Region{{Name: "us-east1", Endpoint: "fakeendpoint2"}}, - Config: map[string]interface{}{"operator-storage": "operator-sc", "workload-storage": ""}, + Config: map[string]interface{}{"workload-storage": "workload-sc"}, RegionConfig: cloud.RegionConfig(nil), CACertificates: []string{"fakecadata2"}, }, @@ -790,7 +790,7 @@ type testData struct { func (s *addCAASSuite) assertAddCloudResult( c *gc.C, testRun func(), - cloudRegion, workloadStorage, operatorStorage string, + cloudRegion, workloadStorage string, t testData, ) { @@ -826,7 +826,7 @@ func (s *addCAASSuite) assertAddCloudResult( Endpoint: "https://1.1.1.1:8888", IdentityEndpoint: "", StorageEndpoint: "", - Config: map[string]interface{}{"operator-storage": operatorStorage, "workload-storage": workloadStorage}, + Config: map[string]interface{}{"workload-storage": workloadStorage}, RegionConfig: cloud.RegionConfig(nil), CACertificates: []string{"A"}, } @@ -887,7 +887,7 @@ func (s *addCAASSuite) TestGatherClusterRegionMetaRegionMatchesAndPassThrough(c c.Assert(err, jc.ErrorIsNil) c.Assert(strings.Trim(cmdtesting.Stdout(ctx), "\n"), gc.Equals, `k8s substrate "myk8s" added as cloud "myk8s". You can now bootstrap to this cloud by running 'juju bootstrap myk8s'.`) - }, cloudRegion, "", "operator-sc", testData{client: true, controller: true}) + }, cloudRegion, "workload-sc", testData{client: true, controller: true}) } func (s *addCAASSuite) TestGatherClusterMetadataError(c *gc.C) { @@ -914,9 +914,6 @@ func (s *addCAASSuite) TestGatherClusterMetadataNoRegions(c *gc.C) { defer ctrl.Finish() result := &k8s.ClusterMetadata{ - OperatorStorageClass: &storagev1.StorageClass{ - ObjectMeta: meta.ObjectMeta{Name: "mystorage"}, - }, WorkloadStorageClass: &storagev1.StorageClass{ ObjectMeta: meta.ObjectMeta{Name: "mystorage"}, }, @@ -932,42 +929,7 @@ func (s *addCAASSuite) TestGatherClusterMetadataNoRegions(c *gc.C) { c.Assert(err, jc.ErrorIsNil) c.Assert(strings.Trim(cmdtesting.Stdout(ctx), "\n"), gc.Equals, `k8s substrate "myk8s" added as cloud "myk8s". You can now bootstrap to this cloud by running 'juju bootstrap myk8s'.`) - }, "other", "mystorage", "mystorage", testData{client: true, controller: true}) -} - -func (s *addCAASSuite) TestGatherClusterMetadataUnknownError(c *gc.C) { - result := &k8s.ClusterMetadata{ - Cloud: "foo", - Regions: set.NewStrings("region"), - } - s.fakeK8sClusterMetadataChecker.Call("GetClusterMetadata").Returns(result, nil) - s.fakeK8sClusterMetadataChecker.Call("CheckDefaultWorkloadStorage").Returns(errors.NotFoundf("foo")) - - err := SetKubeConfigData(kubeConfigStr) - c.Assert(err, jc.ErrorIsNil) - - command := s.makeCommand(c, true, false, true) - _, err = s.runCommand(c, nil, command, "myk8s", "--cluster-name", "myk8s", "-c", "foo") - c.Assert(err, gc.ErrorMatches, ` No recommended storage configuration is defined on this cluster. - Run add-k8s again with --storage= and Juju will use the - specified storage class. -`) -} - -func (s *addCAASSuite) TestGatherClusterMetadataNoStorageError(c *gc.C) { - result := &k8s.ClusterMetadata{} - s.fakeK8sClusterMetadataChecker.Call("GetClusterMetadata").Returns(result, nil) - s.fakeK8sClusterMetadataChecker.Call("CheckDefaultWorkloadStorage").Returns(errors.NotFoundf("foo")) - - err := SetKubeConfigData(kubeConfigStr) - c.Assert(err, jc.ErrorIsNil) - - command := s.makeCommand(c, true, false, true) - _, err = s.runCommand(c, nil, command, "myk8s", "--cluster-name", "myk8s", "-c", "foo") - c.Assert(err, gc.ErrorMatches, ` No recommended storage configuration is defined on this cluster. - Run add-k8s again with --storage= and Juju will use the - specified storage class. -`) + }, "other", "mystorage", testData{client: true, controller: true}) } func (s *addCAASSuite) TestGatherClusterMetadataUserStorage(c *gc.C) { @@ -975,9 +937,6 @@ func (s *addCAASSuite) TestGatherClusterMetadataUserStorage(c *gc.C) { defer ctrl.Finish() result := &k8s.ClusterMetadata{ - OperatorStorageClass: &storagev1.StorageClass{ - ObjectMeta: meta.ObjectMeta{Name: "mystorage"}, - }, WorkloadStorageClass: &storagev1.StorageClass{ ObjectMeta: meta.ObjectMeta{Name: "mystorage"}, }, @@ -994,24 +953,7 @@ func (s *addCAASSuite) TestGatherClusterMetadataUserStorage(c *gc.C) { c.Assert(strings.Trim(cmdtesting.Stdout(ctx), "\n"), gc.Equals, `k8s substrate "myk8s" added as cloud "myk8s" with storage provisioned by the existing "mystorage" storage class. You can now bootstrap to this cloud by running 'juju bootstrap myk8s'.`) - }, "other", "mystorage", "mystorage", testData{client: true, controller: true}) -} - -func (s *addCAASSuite) TestGatherClusterMetadataNoRecommendedStorageError(c *gc.C) { - result := k8s.ClusterMetadata{} - s.fakeK8sClusterMetadataChecker.Call("GetClusterMetadata").Returns(&result, nil) - - err := SetKubeConfigData(kubeConfigStr) - c.Assert(err, jc.ErrorIsNil) - - command := s.makeCommand(c, true, false, true) - _, err = s.runCommand(c, nil, command, "myk8s", "--cluster-name", "myk8s", "-c", "foo") - expectedErr := ` - No recommended storage configuration is defined on this cluster. - Run add-k8s again with --storage= and Juju will use the - specified storage class. -`[1:] - c.Assert(err, gc.ErrorMatches, expectedErr) + }, "other", "mystorage", testData{client: true, controller: true}) } func (s *addCAASSuite) TestUnknownClusterExistingStorageClass(c *gc.C) { @@ -1023,9 +965,6 @@ func (s *addCAASSuite) TestUnknownClusterExistingStorageClass(c *gc.C) { defaultClusterMetadata := &k8s.ClusterMetadata{ Cloud: cloudRegion, - OperatorStorageClass: &storagev1.StorageClass{ - ObjectMeta: meta.ObjectMeta{Name: "mystorage"}, - }, WorkloadStorageClass: &storagev1.StorageClass{ ObjectMeta: meta.ObjectMeta{Name: "mystorage"}, }, @@ -1042,7 +981,7 @@ func (s *addCAASSuite) TestUnknownClusterExistingStorageClass(c *gc.C) { result := strings.Trim(cmdtesting.Stdout(ctx), "\n") result = strings.Replace(result, "\n", " ", -1) c.Assert(result, gc.Equals, `k8s substrate "myk8s" added as cloud "myk8s" with storage provisioned by the existing "mystorage" storage class. You can now bootstrap to this cloud by running 'juju bootstrap myk8s'.`) - }, cloudRegion, "mystorage", "mystorage", testData{client: true, controller: true}) + }, cloudRegion, "mystorage", testData{client: true, controller: true}) } @@ -1072,7 +1011,6 @@ func (s *addCAASSuite) TestFoundStorageProvisionerViaAnnationForMAASWIthoutStora } s.fakeK8sClusterMetadataChecker.Call("GetClusterMetadata").Returns(&k8s.ClusterMetadata{ Cloud: "maas", - OperatorStorageClass: sc, WorkloadStorageClass: sc, }, nil) @@ -1086,7 +1024,7 @@ func (s *addCAASSuite) TestFoundStorageProvisionerViaAnnationForMAASWIthoutStora result := strings.Trim(cmdtesting.Stdout(ctx), "\n") result = strings.Replace(result, "\n", " ", -1) c.Assert(result, gc.Equals, `k8s substrate "myk8s" added as cloud "myk8s". You can now bootstrap to this cloud by running 'juju bootstrap myk8s'.`) - }, "maas", "mystorage", "mystorage", testData{client: true, controller: true}) + }, "maas", "mystorage", testData{client: true, controller: true}) } func (s *addCAASSuite) TestLocalOnly(c *gc.C) { @@ -1101,9 +1039,6 @@ func (s *addCAASSuite) TestLocalOnly(c *gc.C) { defaultClusterMetadata := &k8s.ClusterMetadata{ Cloud: cloudRegion, - OperatorStorageClass: &storagev1.StorageClass{ - ObjectMeta: meta.ObjectMeta{Name: "operator-sc"}, - }, } s.fakeK8sClusterMetadataChecker.Call("GetClusterMetadata").Returns(defaultClusterMetadata, nil) @@ -1113,7 +1048,7 @@ func (s *addCAASSuite) TestLocalOnly(c *gc.C) { c.Assert(err, jc.ErrorIsNil) expected := `k8s substrate "myk8s" added as cloud "myk8s".You can now bootstrap to this cloud by running 'juju bootstrap myk8s'.` c.Assert(strings.Replace(cmdtesting.Stdout(ctx), "\n", "", -1), gc.Equals, expected) - }, cloudRegion, "", "operator-sc", testData{client: true}) + }, cloudRegion, "", testData{client: true}) } func mockStdinPipe(content string) (*os.File, error) { @@ -1259,8 +1194,7 @@ func (s *addCAASSuite) assertStoreClouds(c *gc.C, hostCloud string) { {Name: "us-east1", Endpoint: "https://1.1.1.1:8888"}, }, Config: map[string]interface{}{ - "operator-storage": "operator-sc", - "workload-storage": "", + "workload-storage": "workload-sc", }, RegionConfig: cloud.RegionConfig(nil), CACertificates: []string{"A"}, @@ -1341,7 +1275,7 @@ func (s *addCAASSuite) TestCorrectUseCurrentContext(c *gc.C) { IdentityEndpoint: "", StorageEndpoint: "", Regions: []cloud.Region{{Name: "us-east1", Endpoint: "https://1.1.1.1:8888"}}, - Config: map[string]interface{}{"operator-storage": "operator-sc", "workload-storage": ""}, + Config: map[string]interface{}{"workload-storage": "workload-sc"}, RegionConfig: cloud.RegionConfig(nil), CACertificates: []string{"A"}, }, @@ -1396,7 +1330,7 @@ func (s *addCAASSuite) TestCorrectSelectContext(c *gc.C) { IdentityEndpoint: "", StorageEndpoint: "", Regions: []cloud.Region{{Name: "us-east1", Endpoint: "https://1.1.1.1:8888"}}, - Config: map[string]interface{}{"operator-storage": "operator-sc", "workload-storage": ""}, + Config: map[string]interface{}{"workload-storage": "workload-sc"}, RegionConfig: cloud.RegionConfig(nil), CACertificates: []string{"A"}, }, diff --git a/cmd/juju/caas/update_test.go b/cmd/juju/caas/update_test.go index 9b20370c2bb..431d2c732a4 100644 --- a/cmd/juju/caas/update_test.go +++ b/cmd/juju/caas/update_test.go @@ -90,8 +90,7 @@ clouds: ca-certificates: - fakecadata2 config: - operator-storage: operator-sc - workload-storage: "" + workload-storage: workload-sc mrcloud1: type: maas description: Metal As A Service @@ -113,8 +112,8 @@ func (s *updateCAASSuite) SetUpTest(c *gc.C) { defaultClusterMetadata := &k8s.ClusterMetadata{ Cloud: "gce", Regions: set.NewStrings("us-east1"), - OperatorStorageClass: &storagev1.StorageClass{ - ObjectMeta: meta.ObjectMeta{Name: "operator-sc"}, + WorkloadStorageClass: &storagev1.StorageClass{ + ObjectMeta: meta.ObjectMeta{Name: "workload-sc"}, }, } s.fakeK8sClusterMetadataChecker = &fakeK8sClusterMetadataChecker{ @@ -174,7 +173,7 @@ func (s *updateCAASSuite) TestCloudNotFound(c *gc.C) { func (s *updateCAASSuite) assertUpdateCloudResult( c *gc.C, testRun func(), - cloudName, cloudRegion, workloadStorage, operatorStorage string, + cloudName, cloudRegion, workloadStorage string, t testData, ) { @@ -192,7 +191,7 @@ func (s *updateCAASSuite) assertUpdateCloudResult( Endpoint: "https://6.6.6.6:8888", IdentityEndpoint: "", StorageEndpoint: "", - Config: map[string]interface{}{"operator-storage": operatorStorage, "workload-storage": workloadStorage}, + Config: map[string]interface{}{"workload-storage": workloadStorage}, RegionConfig: cloud.RegionConfig(nil), CACertificates: []string{"fakecadata2"}, } @@ -244,7 +243,7 @@ func (s *updateCAASSuite) TestLocalOnly(c *gc.C) { c.Assert(err, jc.ErrorIsNil) expected := `k8s cloud "myk8s" updated on this client.` c.Assert(strings.Replace(cmdtesting.Stderr(ctx), "\n", "", -1), gc.Equals, expected) - }, "myk8s", "gce/us-east1", "", "operator-sc", testData{client: true}) + }, "myk8s", "gce/us-east1", "workload-sc", testData{client: true}) } func (s *updateCAASSuite) TestInvalidControllerCloud(c *gc.C) { @@ -271,7 +270,7 @@ func (s *updateCAASSuite) TestControllerAndClient(c *gc.C) { c.Assert(err, jc.ErrorIsNil) c.Assert(strings.Replace(cmdtesting.Stderr(ctx), "\n", "", -1), gc.Equals, `k8s cloud "myk8s" updated on this client.k8s cloud "myk8s" updated on controller "foo".`) - }, "myk8s", "gce/us-east1", "", "operator-sc", testData{client: true, controller: true}) + }, "myk8s", "gce/us-east1", "workload-sc", testData{client: true, controller: true}) } func (s *updateCAASSuite) TestBuiltinLocal(c *gc.C) { diff --git a/cmd/juju/storage/poolupdate.go b/cmd/juju/storage/poolupdate.go index 799f86109db..3d3337343ca 100644 --- a/cmd/juju/storage/poolupdate.go +++ b/cmd/juju/storage/poolupdate.go @@ -31,7 +31,7 @@ Update configuration attributes for a single existing storage pool. const poolUpdateCommandExamples = ` Update the storage-pool named iops with new configuration details: - juju update-storage-pool operator-storage volume-type=provisioned-iops iops=40 + juju update-storage-pool workload-storage volume-type=provisioned-iops iops=40 Update which provider the pool is for: diff --git a/domain/modelconfig/modelmigration/import.go b/domain/modelconfig/modelmigration/import.go index 04c0604c9fa..67a91b30a58 100644 --- a/domain/modelconfig/modelmigration/import.go +++ b/domain/modelconfig/modelmigration/import.go @@ -6,9 +6,11 @@ package modelmigration import ( "context" + "github.com/juju/collections/set" "github.com/juju/description/v8" "github.com/juju/errors" + "github.com/juju/juju/core/logger" "github.com/juju/juju/core/modelmigration" "github.com/juju/juju/domain/modelconfig/service" "github.com/juju/juju/domain/modelconfig/state" @@ -22,9 +24,12 @@ type Coordinator interface { } // RegisterImport registers the import operations with the given coordinator. -func RegisterImport(coordinator Coordinator, defaultsProvider service.ModelDefaultsProvider) { +func RegisterImport( + coordinator Coordinator, defaultsProvider service.ModelDefaultsProvider, logger logger.Logger, +) { coordinator.Add(&importOperation{ defaultsProvider: defaultsProvider, + logger: logger, }) } @@ -42,6 +47,7 @@ type ImportService interface { type importOperation struct { modelmigration.BaseOperation + logger logger.Logger service ImportService defaultsProvider service.ModelDefaultsProvider } @@ -73,6 +79,25 @@ func (i *importOperation) Execute(ctx context.Context, model description.Model) return errors.NotValidf("model config") } + // Models imported from older controllers may contain config attributes + // which have since been removed from use. We filter these out by removing + // any incoming attributes not in the default list. + defaults, err := i.defaultsProvider.ModelDefaults(ctx) + if err != nil { + return errors.Trace(err) + } + defaultAttrs := set.NewStrings() + for k := range defaults { + defaultAttrs.Add(k) + } + + for k, v := range attrs { + if !defaultAttrs.Contains(k) { + i.logger.Debugf("model config attribute %s=%v is removed on import", k, v) + delete(attrs, k) + } + } + if err := i.service.SetModelConfig(ctx, attrs); err != nil { return errors.Trace(err) } diff --git a/domain/modelconfig/modelmigration/import_test.go b/domain/modelconfig/modelmigration/import_test.go index b3498b4547f..d96b1926167 100644 --- a/domain/modelconfig/modelmigration/import_test.go +++ b/domain/modelconfig/modelmigration/import_test.go @@ -12,7 +12,9 @@ import ( "go.uber.org/mock/gomock" gc "gopkg.in/check.v1" + "github.com/juju/juju/domain/modeldefaults" "github.com/juju/juju/environs/config" + loggertesting "github.com/juju/juju/internal/logger/testing" ) type importSuite struct { @@ -38,7 +40,7 @@ func (s *importSuite) TestRegisterImport(c *gc.C) { s.coordinator.EXPECT().Add(gomock.Any()) - RegisterImport(s.coordinator, s.modelDefaultsProvider) + RegisterImport(s.coordinator, s.modelDefaultsProvider, loggertesting.WrapCheckLog(c)) } func (s *importSuite) TestEmptyModelConfig(c *gc.C) { @@ -47,7 +49,7 @@ func (s *importSuite) TestEmptyModelConfig(c *gc.C) { // Empty model. model := description.NewModel(description.ModelArgs{}) - op := s.newImportOperation() + op := s.newImportOperation(c) err := op.Execute(context.Background(), model) c.Assert(err, jc.ErrorIs, errors.NotValid) } @@ -56,26 +58,37 @@ func (s *importSuite) TestModelConfig(c *gc.C) { defer s.setupMocks(c).Finish() config, err := config.New(config.NoDefaults, map[string]any{ - "name": "foo", - "uuid": "a677bdfd-3c96-46b2-912f-38e25faceaf7", - "type": "sometype", + "name": "foo", + "uuid": "a677bdfd-3c96-46b2-912f-38e25faceaf7", + "type": "sometype", + "workload-storage": "mystorage", + "operator-storage": "otherstorage", }) c.Assert(err, jc.ErrorIsNil) + importedCOnfig := map[string]any{ + "logging-config": "=INFO", + "workload-storage": "mystorage", + } - s.service.EXPECT().SetModelConfig(gomock.Any(), config.AllAttrs()).Return(nil) + s.service.EXPECT().SetModelConfig(gomock.Any(), importedCOnfig).Return(nil) + s.modelDefaultsProvider.EXPECT().ModelDefaults(gomock.Any()).Return(modeldefaults.Defaults{ + "logging-config": modeldefaults.DefaultAttributeValue{}, + "workload-storage": modeldefaults.DefaultAttributeValue{}, + }, nil) model := description.NewModel(description.ModelArgs{ Config: config.AllAttrs(), }) - op := s.newImportOperation() + op := s.newImportOperation(c) err = op.Execute(context.Background(), model) c.Assert(err, jc.ErrorIsNil) } -func (s *importSuite) newImportOperation() *importOperation { +func (s *importSuite) newImportOperation(c *gc.C) *importOperation { return &importOperation{ service: s.service, defaultsProvider: s.modelDefaultsProvider, + logger: loggertesting.WrapCheckLog(c), } } diff --git a/domain/modelmigration/import.go b/domain/modelmigration/import.go index e966e76de31..39f79a8bb2e 100644 --- a/domain/modelmigration/import.go +++ b/domain/modelmigration/import.go @@ -53,7 +53,7 @@ func ImportOperations( credential.RegisterImport(coordinator, logger.Child("credential")) model.RegisterImport(coordinator, logger.Child("model")) keymanager.RegisterImport(coordinator, logger.Child("keymanager")) - modelconfig.RegisterImport(coordinator, modelDefaultsProvider) + modelconfig.RegisterImport(coordinator, modelDefaultsProvider, logger.Child("modelconfig")) access.RegisterImport(coordinator, logger.Child("access")) network.RegisterImport(coordinator, logger.Child("network")) machine.RegisterImport(coordinator, logger.Child("machine"))