Skip to content

Commit

Permalink
Remove Extra Fields From Minimal CR (#800)
Browse files Browse the repository at this point in the history 
* set default replicas to 2 if not specified

* set default path for KUBELET_CONFIG_DIR

* set default path for KUBELET_CONFIG_DIR

* define DefaultKubeletConfigDir

* Same KUBECONFIG_DIR changes for powerscale and powerstore. Tested on
K8S.

* updated structs to pointers

* Remove mdm from CR. We still need to add the MDM var to the daemonset.

* fixed operator crashing

* [WIP] Increase coverage for driver package (#801)

* Update common_test.go to increase powerscale.go coverage

* Update powerscale_test.go

* Fixed initContainers in CR

* Update common_test.go for increaing powermax.go coverage

* Update common_test.go

* Update common_test.go to increase powerflex.go coverage

* fixed linting errors

* fixed linting errors

* fixed yaml linting errors

* To increase coverage for powerstore.go

* Increase coverage for unity.go file

* Increase coverage for unity.go file

* To increase coverage for unity.go

* To increase coverage for commonconfig.go

* To increase coverage for commonconfig.go

* Fixed Linting issues

* Fixed Linting issues

* Fixed Linting issues

* fixed linters error

* fixed linters error

* e2e tests for minimal sample

* Removing code that defaults replica count in deployment and CR to 2 when
it comes in as 0.

* Fix linter error.

* addressed @rajendraindukuri & @kumarp20 review comment

* fixed liniting errors for crd

* added no mdm unit test

* fixed reverseproxy envs

* Inject ReverseProxy directly into the deployment

* Refactor ReverseProxy code

* Fix unit test failures

* Fix step_defs.go

* Increase coverage of controller

* Increase coverage of controller

* Fix gosec error

* Fix lint error

* Increase coverage of reverseproxy

* Fix lint error

* Fix minimal e2e scenarios

* Fix driver UT

---------

Co-authored-by: Surya Gupta <[email protected]>
Co-authored-by: Rishabh Raj <[email protected]>
Co-authored-by: sakshi-garg1 <[email protected]>
Co-authored-by: Akshay Saini <[email protected]>
Co-authored-by: Akshay Saini <[email protected]>
  • Loading branch information
@anathoodell @suryagupta4
@rishabhatdell @sakshi-garg1 @AkshaySainiDell
6 people authored Dec 20, 2024
1 parent 91165ff commit 498b3f9
Show file tree
Hide file tree
Showing 28 changed files with 2,731 additions and 1,831 deletions.
9 changes: 5 additions & 4 deletions api/v1/types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -177,14 +177,15 @@ type Driver struct {

// CSIDriverSpec is the specification for CSIDriver
// +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="CSI Driver Spec"
CSIDriverSpec CSIDriverSpec `json:"csiDriverSpec" yaml:"csiDriverSpec"`
CSIDriverSpec *CSIDriverSpec `json:"csiDriverSpec" yaml:"csiDriverSpec"`

// ConfigVersion is the configuration version of the driver
// +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Config Version"
ConfigVersion string `json:"configVersion" yaml:"configVersion"`

// Replicas is the count of controllers for Controller plugin
// +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Controller count"
// +kubebuilder:default=2
Replicas int32 `json:"replicas" yaml:"replicas"`

// DNSPolicy is the dnsPolicy of the daemonset for Node plugin
Expand All @@ -193,15 +194,15 @@ type Driver struct {

// Common is the common specification for both controller and node plugins
// +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Common specification"
Common ContainerTemplate `json:"common" yaml:"common"`
Common *ContainerTemplate `json:"common" yaml:"common"`

// Controller is the specification for Controller plugin only
// +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Controller Specification"
Controller ContainerTemplate `json:"controller,omitempty" yaml:"controller"`
Controller *ContainerTemplate `json:"controller,omitempty" yaml:"controller"`

// Node is the specification for Node plugin only
// +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Node specification"
Node ContainerTemplate `json:"node,omitempty" yaml:"node"`
Node *ContainerTemplate `json:"node,omitempty" yaml:"node"`

// SideCars is the specification for CSI sidecar containers
// +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="CSI SideCars specification"
Expand Down
24 changes: 20 additions & 4 deletions api/v1/zz_generated.deepcopy.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2,895 changes: 1,711 additions & 1,184 deletions config/crd/bases/storage.dell.com_containerstoragemodules.yaml
View file

Large diffs are not rendered by default.

54 changes: 35 additions & 19 deletions controllers/csm_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -737,15 +737,15 @@ func (r *ContainerStorageModuleReconciler) SyncCSM(ctx context.Context, cr csmv1
}

// Create/Update Reverseproxy Server
if reverseProxyEnabled, _ := utils.IsModuleEnabled(ctx, cr, csmv1.ReverseProxy); reverseProxyEnabled {
if reverseProxyEnabled, _ := utils.IsModuleEnabled(ctx, cr, csmv1.ReverseProxy); reverseProxyEnabled && !modules.IsReverseProxySidecar() {
log.Infow("Trying Create/Update reverseproxy...")
if err := r.reconcileReverseProxy(ctx, false, operatorConfig, cr, ctrlClient); err != nil {
if err := r.reconcileReverseProxyServer(ctx, false, operatorConfig, cr, ctrlClient); err != nil {
return fmt.Errorf("failed to deploy reverseproxy proxy server: %v", err)
}
}

// Get Driver resources
driverConfig, err := getDriverConfig(ctx, cr, operatorConfig)
driverConfig, err := getDriverConfig(ctx, cr, operatorConfig, ctrlClient)
if err != nil {
return err
}
Expand All @@ -765,6 +765,25 @@ func (r *ContainerStorageModuleReconciler) SyncCSM(ctx context.Context, cr csmv1
node := driverConfig.Node
controller := driverConfig.Controller

if cr.GetDriverType() == csmv1.PowerMax {
if !modules.IsReverseProxySidecar() {
log.Infof("DeployAsSidar is false...csi-reverseproxy should be present as deployement\n")
log.Infof("adding proxy service name...\n")
modules.AddReverseProxyServiceName(&controller.Deployment)
} else {
log.Info("Starting CSI ReverseProxy Service")
if err := modules.ReverseProxyStartService(ctx, false, operatorConfig, cr, ctrlClient); err != nil {
return fmt.Errorf("unable to reconcile reverse-proxy service: %v", err)
}
log.Info("Injecting CSI ReverseProxy")
dp, err := modules.ReverseProxyInjectDeployment(controller.Deployment, cr, operatorConfig)
if err != nil {
return fmt.Errorf("injecting replication into deployment: %v", err)
}
controller.Deployment = *dp
}
}

replicationEnabled, clusterClients, err := utils.GetDefaultClusters(ctx, cr, r)
if err != nil {
return err
Expand Down Expand Up @@ -833,13 +852,6 @@ func (r *ContainerStorageModuleReconciler) SyncCSM(ctx context.Context, cr csmv1
}

controller.Rbac.ClusterRole = *clusterRole
case csmv1.ReverseProxy:
log.Info("Injecting CSI ReverseProxy")
dp, err := modules.ReverseProxyInjectDeployment(controller.Deployment, cr, operatorConfig)
if err != nil {
return fmt.Errorf("injecting replication into deployment: %v", err)
}
controller.Deployment = *dp
}
}
}
Expand Down Expand Up @@ -1101,6 +1113,7 @@ func (r *ContainerStorageModuleReconciler) reconcileAppMobility(ctx context.Cont
func getDriverConfig(ctx context.Context,
cr csmv1.ContainerStorageModule,
operatorConfig utils.OperatorConfig,
ctrlClient client.Client,
) (*DriverConfig, error) {
var (
err error
Expand Down Expand Up @@ -1135,7 +1148,7 @@ func getDriverConfig(ctx context.Context,
return nil, fmt.Errorf("getting %s CSIDriver: %v", driverType, err)
}

node, err = drivers.GetNode(ctx, cr, operatorConfig, driverType, NodeYaml)
node, err = drivers.GetNode(ctx, cr, operatorConfig, driverType, NodeYaml, ctrlClient)
if err != nil {
return nil, fmt.Errorf("getting %s node: %v", driverType, err)
}
Expand All @@ -1153,16 +1166,13 @@ func getDriverConfig(ctx context.Context,
}, nil
}

// reconcileReverseProxy - deploy reverse proxy server
func (r *ContainerStorageModuleReconciler) reconcileReverseProxy(ctx context.Context, isDeleting bool, op utils.OperatorConfig, cr csmv1.ContainerStorageModule, ctrlClient client.Client) error {
// reconcileReverseProxyServer - deploy reverse proxy server
func (r *ContainerStorageModuleReconciler) reconcileReverseProxyServer(ctx context.Context, isDeleting bool, op utils.OperatorConfig, cr csmv1.ContainerStorageModule, ctrlClient client.Client) error {
log := logger.GetLogger(ctx)
log.Infow("Reconcile reverseproxy proxy")
if err := modules.ReverseProxyServer(ctx, isDeleting, op, cr, ctrlClient); err != nil {
return fmt.Errorf("unable to reconcile reverse-proxy server: %v", err)
}
if err := modules.ReverseProxyStartService(ctx, isDeleting, op, cr, ctrlClient); err != nil {
return fmt.Errorf("unable to reconcile reverse-proxy service: %v", err)
}
return nil
}

Expand Down Expand Up @@ -1250,7 +1260,7 @@ func (r *ContainerStorageModuleReconciler) removeDriver(ctx context.Context, ins
log := logger.GetLogger(ctx)

// Get Driver resources
driverConfig, err := getDriverConfig(ctx, instance, operatorConfig)
driverConfig, err := getDriverConfig(ctx, instance, operatorConfig, r.Client)
if err != nil {
log.Error("error in getDriverConfig")
return err
Expand Down Expand Up @@ -1287,6 +1297,12 @@ func (r *ContainerStorageModuleReconciler) removeDriver(ctx context.Context, ins
}
}

if instance.GetDriverType() == csmv1.PowerMax && modules.IsReverseProxySidecar() {
log.Info("Removing CSI ReverseProxy Service")
if err := modules.ReverseProxyStartService(ctx, true, operatorConfig, instance, cluster.ClusterCTRLClient); err != nil {
return fmt.Errorf("unable to reconcile reverse-proxy service: %v", err)
}
}
}

return nil
Expand All @@ -1309,9 +1325,9 @@ func (r *ContainerStorageModuleReconciler) removeModule(ctx context.Context, ins
return err
}
}
if reverseproxyEnabled, _ := utils.IsModuleEnabled(ctx, instance, csmv1.ReverseProxy); reverseproxyEnabled {
if reverseproxyEnabled, _ := utils.IsModuleEnabled(ctx, instance, csmv1.ReverseProxy); reverseproxyEnabled && !modules.IsReverseProxySidecar() {
log.Infow("Deleting ReverseProxy")
if err := r.reconcileReverseProxy(ctx, true, operatorConfig, instance, ctrlClient); err != nil {
if err := r.reconcileReverseProxyServer(ctx, true, operatorConfig, instance, ctrlClient); err != nil {
return err
}
}
Expand Down
24 changes: 20 additions & 4 deletions controllers/csm_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,17 @@ func (suite *CSMControllerTestSuite) TestReverseProxyReconcile() {
suite.runFakeCSMManager("", true)
}

func (suite *CSMControllerTestSuite) TestReverseProxySidecarReconcile() {
revProxy := getReverseProxyModule()
deploAsSidecar := corev1.EnvVar{Name: "DeployAsSidecar", Value: "true"}
revProxy[0].Components[0].Envs = append(revProxy[0].Components[0].Envs, deploAsSidecar)
modules.IsReverseProxySidecar = func() bool { return true }
suite.makeFakeRevProxyCSM(csmName, suite.namespace, true, revProxy, string(v1.PowerMax))
suite.runFakeCSMManager("", false)
suite.deleteCSM(csmName)
suite.runFakeCSMManager("", true)
}

func (suite *CSMControllerTestSuite) TestReverseProxyPreCheckError() {
suite.makeFakeRevProxyCSM(csmName, suite.namespace, false, getReverseProxyModule(), "badVersion")
reconciler := suite.createReconciler()
Expand All @@ -287,7 +298,7 @@ func (suite *CSMControllerTestSuite) TestReconcileReverseProxyError() {
csm := shared.MakeCSM(csmName, suite.namespace, shared.PmaxConfigVersion)
csm.Spec.Modules = getReverseProxyModule()
reconciler := suite.createReconciler()
err := reconciler.reconcileReverseProxy(ctx, false, badOperatorConfig, csm, suite.fakeClient)
err := reconciler.reconcileReverseProxyServer(ctx, false, badOperatorConfig, csm, suite.fakeClient)
assert.NotNil(suite.T(), err)
}

Expand All @@ -300,7 +311,7 @@ func (suite *CSMControllerTestSuite) TestReconcileReverseProxyServiceError() {
reconciler := suite.createReconciler()
_ = modules.ReverseProxyPrecheck(ctx, operatorConfig, revProxy[0], csm, reconciler)
revProxy[0].ConfigVersion = ""
err := reconciler.reconcileReverseProxy(ctx, false, badOperatorConfig, csm, suite.fakeClient)
err := reconciler.reconcileReverseProxyServer(ctx, false, badOperatorConfig, csm, suite.fakeClient)
assert.NotNil(suite.T(), err)
}

Expand Down Expand Up @@ -738,7 +749,8 @@ func (suite *CSMControllerTestSuite) TestRemoveDriver() {
csmBadType.Spec.Driver.CSIDriverType = "wrongdriver"
csmWoType := shared.MakeCSM(csmName, suite.namespace, configVersion)
csm := shared.MakeCSM(csmName, suite.namespace, configVersion)
csm.Spec.Driver.CSIDriverType = "powerscale"
csm.Spec.Driver.CSIDriverType = csmv1.PowerMax
modules.IsReverseProxySidecar = func() bool { return true }

removeDriverTests := []struct {
name string
Expand Down Expand Up @@ -801,6 +813,7 @@ func (suite *CSMControllerTestSuite) TestSyncCSM() {
appMobCSM.Spec.Modules = getAppMob()
reverseProxyServerCSM := shared.MakeCSM(csmName, suite.namespace, configVersion)
reverseProxyServerCSM.Spec.Modules = getReverseProxyModule()
modules.IsReverseProxySidecar = func() bool { return false }

syncCSMTests := []struct {
name string
Expand Down Expand Up @@ -861,6 +874,9 @@ func (suite *CSMControllerTestSuite) TestRemoveModule() {
}
*tt.errorInjector = true
}
if tt.csm.HasModule(csmv1.ReverseProxy) {
modules.IsReverseProxySidecar = func() bool { return false }
}
err := r.removeModule(ctx, tt.csm, operatorConfig, r.Client)
if tt.expectedErr == "" {
assert.Nil(t, err)
Expand Down Expand Up @@ -926,7 +942,7 @@ func (suite *CSMControllerTestSuite) TestOldStandAloneModuleCleanup() {
if errorInjector != nil {
*errorInjector = true
}
driverConfig, _ := getDriverConfig(ctx, *csm, operatorConfig)
driverConfig, _ := getDriverConfig(ctx, *csm, operatorConfig, r.Client)
err := r.oldStandAloneModuleCleanup(ctx, csm, operatorConfig, driverConfig)

if expectedErr == "" {
Expand Down
Loading

0 comments on commit 498b3f9

Please sign in to comment.