Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ go 1.19

require (
github.com/blang/semver v3.5.1+incompatible
github.com/openshift/api v0.0.0-20230120195050-6ba31fa438f2
github.com/openshift/api v0.0.0-20230330150608-05635858d40f
github.com/openshift/build-machinery-go v0.0.0-20220913142420-e25cf57ea46d
github.com/openshift/client-go v0.0.0-20230120202327-72f107311084
github.com/openshift/library-go v0.0.0-20230130232623-47904dd9ff5a
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -416,8 +416,8 @@ github.com/onsi/ginkgo/v2 v2.4.0 h1:+Ig9nvqgS5OBSACXNk15PLdp0U9XPYROt9CFzVdFGIs=
github.com/onsi/gomega v0.0.0-20170829124025-dcabb60a477c/go.mod h1:C1qb7wdrVGGVU+Z6iS04AVkA3Q65CEZX59MT0QO5uiA=
github.com/onsi/gomega v1.7.0/go.mod h1:ex+gbHU/CVuBBDIJjb2X0qEXbFg53c61hWP/1CpauHY=
github.com/onsi/gomega v1.23.0 h1:/oxKu9c2HVap+F3PfKort2Hw5DEU+HGlW8n+tguWsys=
github.com/openshift/api v0.0.0-20230120195050-6ba31fa438f2 h1:+nw0/d4spq880W7S74Twi5YU2ulsl3/a9o4OEZptYp0=
github.com/openshift/api v0.0.0-20230120195050-6ba31fa438f2/go.mod h1:ctXNyWanKEjGj8sss1KjjHQ3ENKFm33FFnS5BKaIPh4=
github.com/openshift/api v0.0.0-20230330150608-05635858d40f h1:mGpCtfoehMcvmg/sSYLiv6nCbTl04cmtkUfYzP7H1AQ=
github.com/openshift/api v0.0.0-20230330150608-05635858d40f/go.mod h1:ctXNyWanKEjGj8sss1KjjHQ3ENKFm33FFnS5BKaIPh4=
github.com/openshift/build-machinery-go v0.0.0-20220913142420-e25cf57ea46d h1:RR4ah7FfaPR1WePizm0jlrsbmPu91xQZnAsVVreQV1k=
github.com/openshift/build-machinery-go v0.0.0-20220913142420-e25cf57ea46d/go.mod h1:b1BuldmJlbA/xYtdZvKi+7j5YGB44qJUJDZ9zwiNCfE=
github.com/openshift/client-go v0.0.0-20230120202327-72f107311084 h1:66uaqNwA+qYyQDwsMWUfjjau8ezmg1dzCqub13KZOcE=
Expand Down
1 change: 1 addition & 0 deletions pkg/operator/starter.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ func RunOperator(ctx context.Context, controllerConfig *controllercmd.Controller
ConfigClientSet: configClient,
ConfigInformers: configInformers,
DynamicClient: dynamicClient,
OCPOperatorInformers: ocpOperatorInformer,
ClusterCSIDriverInformer: clusterCSIDriverInformer,
}

Expand Down
19 changes: 19 additions & 0 deletions pkg/operator/testlib/testlib.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ var f embed.FS
const (
cloudConfigNamespace = "openshift-config"
infraGlobalName = "cluster"
storageOperatorName = "cluster"
secretName = "vmware-vsphere-cloud-credentials"
defaultNamespace = "openshift-cluster-csi-drivers"
)
Expand Down Expand Up @@ -147,6 +148,9 @@ func AddInitialObjects(objects []runtime.Object, clients *utils.APIClient) error
case *opv1.ClusterCSIDriver:
clusterCSIDriverInformer := clients.ClusterCSIDriverInformer.Informer()
clusterCSIDriverInformer.GetStore().Add(obj)
case *opv1.Storage:
storageInformer := clients.OCPOperatorInformers.Operator().V1().Storages().Informer()
storageInformer.GetStore().Add(obj)
default:
return fmt.Errorf("Unknown initalObject type: %+v", obj)
}
Expand Down Expand Up @@ -255,6 +259,21 @@ func GetInfraObject() *ocpv1.Infrastructure {
}
}

func GetStorageOperator(driver opv1.StorageDriverType) *opv1.Storage {
return &opv1.Storage{
ObjectMeta: metav1.ObjectMeta{
Name: storageOperatorName,
},
Spec: opv1.StorageSpec{
OperatorSpec: opv1.OperatorSpec{
ManagementState: opv1.Managed,
},
VSphereStorageDriver: driver,
},
Status: opv1.StorageStatus{},
}
}

func GetSingleFailureDomainInfra() *ocpv1.Infrastructure {
return &ocpv1.Infrastructure{
ObjectMeta: metav1.ObjectMeta{
Expand Down
8 changes: 8 additions & 0 deletions pkg/operator/vspherecontroller/checks/api_interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package checks

import (
ocpv1 "github.com/openshift/api/config/v1"
operatorv1 "github.com/openshift/api/operator/v1"
oplister "github.com/openshift/client-go/operator/listers/operator/v1"
v1 "k8s.io/api/core/v1"
storagev1 "k8s.io/api/storage/v1"
"k8s.io/apimachinery/pkg/labels"
Expand All @@ -16,6 +18,7 @@ type KubeAPIInterface interface {
ListCSINodes() ([]*storagev1.CSINode, error)
GetStorageClass(name string) (*storagev1.StorageClass, error)
GetInfrastructure() *ocpv1.Infrastructure
GetStorage(name string) (*operatorv1.Storage, error)
}

type KubeAPIInterfaceImpl struct {
Expand All @@ -24,6 +27,7 @@ type KubeAPIInterfaceImpl struct {
CSINodeLister storagelister.CSINodeLister
CSIDriverLister storagelister.CSIDriverLister
StorageClassLister storagelister.StorageClassLister
StorageLister oplister.StorageLister
}

func (k *KubeAPIInterfaceImpl) ListNodes() ([]*v1.Node, error) {
Expand All @@ -45,3 +49,7 @@ func (k *KubeAPIInterfaceImpl) GetStorageClass(name string) (*storagev1.StorageC
func (k *KubeAPIInterfaceImpl) GetInfrastructure() *ocpv1.Infrastructure {
return k.Infrastructure
}

func (k *KubeAPIInterfaceImpl) GetStorage(name string) (*operatorv1.Storage, error) {
return k.StorageLister.Get(name)
}
1 change: 1 addition & 0 deletions pkg/operator/vspherecontroller/checks/check_error.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ const (
CheckStatusDeprecatedESXIVersion CheckStatusType = "check_deprecated_esxi_version"
CheckStatusVcenterAPIError CheckStatusType = "vcenter_api_error"
CheckStatusGenericError CheckStatusType = "generic_error"
CheckStatusCSIMigrationDisabled CheckStatusType = "csi_migration_disabled"
)

type ClusterCheckStatus string
Expand Down
34 changes: 34 additions & 0 deletions pkg/operator/vspherecontroller/checks/check_migration_disabled.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package checks

import (
"context"
"fmt"

operatorv1 "github.com/openshift/api/operator/v1"
)

const (
storageOperatorName = "cluster"
)

type CheckMigrationDisabled struct{}

func (c *CheckMigrationDisabled) Check(ctx context.Context, checkOpts CheckArgs) []ClusterCheckResult {
storageOperator, err := checkOpts.apiClient.GetStorage(storageOperatorName)
if err != nil {
checkResult := ClusterCheckResult{
CheckStatus: CheckStatusOpenshiftAPIError,
CheckError: err,
Action: CheckActionDegrade,
Reason: fmt.Sprintf("failed to check Storage operator %s: %v", storageOperatorName, err),
}
return []ClusterCheckResult{checkResult}
}
// If CSI migration is explictly disabled, then block upgrade.
// Future releases have migration enabled and can not be turned off.
if storageOperator.Spec.VSphereStorageDriver == operatorv1.LegacyDeprecatedInTreeDriver {
reason := fmt.Errorf("Storage operator %s must not have vsphereStorageDriver set to LegacyDeprecatedInTreeDriver", storageOperatorName)
return []ClusterCheckResult{MakeClusterUnupgradeableError(CheckStatusCSIMigrationDisabled, reason)}
}
return []ClusterCheckResult{}
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ func newVSphereEnvironmentChecker() *vSphereEnvironmentCheckerComposite {
&checks.CheckExistingDriver{},
&checks.VCenterChecker{},
&checks.NodeChecker{},
&checks.CheckMigrationDisabled{},
}
return checker
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,14 @@ package vspherecontroller

import (
"context"
"github.com/openshift/vmware-vsphere-csi-driver-operator/pkg/operator/testlib"
"os"
"testing"
"time"

"k8s.io/apimachinery/pkg/runtime"

opv1 "github.com/openshift/api/operator/v1"
"github.com/openshift/vmware-vsphere-csi-driver-operator/pkg/operator/testlib"
"github.com/openshift/vmware-vsphere-csi-driver-operator/pkg/operator/vspherecontroller/checks"
)

Expand Down Expand Up @@ -56,10 +57,13 @@ func TestEnvironmentCheck(t *testing.T) {
test := tests[i]
t.Run(test.name, func(t *testing.T) {
commonApiClient := testlib.NewFakeClients(test.initialObjects, test.clusterCSIDriverObject, test.configObjects)
storageOperator := testlib.GetStorageOperator(opv1.CSIWithMigrationDriver)
test.initialObjects = append(test.initialObjects, storageOperator)

stopCh := make(chan struct{})
defer close(stopCh)

go testlib.StartFakeInformer(commonApiClient, stopCh)
testlib.StartFakeInformer(commonApiClient, stopCh)
if err := testlib.AddInitialObjects(test.initialObjects, commonApiClient); err != nil {
t.Fatalf("error adding initial objects: %v", err)
Comment on lines 62 to 68
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a race condition here that didn't show up until I added a new informer. If StartFakeInformer is called by a go routine, the informer may not be started before the initial objects are added. Then the Get call fails later because it can't find the object.

}
Expand Down Expand Up @@ -87,12 +91,14 @@ func TestEnvironmentCheck(t *testing.T) {
csiDriverLister := commonApiClient.KubeInformers.InformersFor("").Storage().V1().CSIDrivers().Lister()
csiNodeLister := commonApiClient.KubeInformers.InformersFor("").Storage().V1().CSINodes().Lister()
nodeLister := commonApiClient.NodeInformer.Lister()
storageLister := commonApiClient.OCPOperatorInformers.Operator().V1().Storages().Lister()

checkerApiClient := &checks.KubeAPIInterfaceImpl{
Infrastructure: testlib.GetInfraObject(),
CSINodeLister: csiNodeLister,
CSIDriverLister: csiDriverLister,
NodeLister: nodeLister,
StorageLister: storageLister,
}
checkOpts := checks.NewCheckArgs(conn, checkerApiClient)
var result checks.ClusterCheckResult
Expand Down
13 changes: 9 additions & 4 deletions pkg/operator/vspherecontroller/vspherecontroller.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ import (

ocpv1 "github.com/openshift/api/config/v1"
operatorapi "github.com/openshift/api/operator/v1"
infralister "github.com/openshift/client-go/config/listers/config/v1"
clustercsidriverlister "github.com/openshift/client-go/operator/listers/operator/v1"
configlister "github.com/openshift/client-go/config/listers/config/v1"
oplister "github.com/openshift/client-go/operator/listers/operator/v1"
"github.com/openshift/library-go/pkg/controller/factory"
"github.com/openshift/library-go/pkg/operator/events"
"github.com/openshift/library-go/pkg/operator/resource/resourceapply"
Expand All @@ -46,8 +46,9 @@ type VSphereController struct {
configMapLister corelister.ConfigMapLister
secretLister corelister.SecretLister
scLister storagelister.StorageClassLister
clusterCSIDriverLister clustercsidriverlister.ClusterCSIDriverLister
infraLister infralister.InfrastructureLister
clusterCSIDriverLister oplister.ClusterCSIDriverLister
storageLister oplister.StorageLister
infraLister configlister.InfrastructureLister
nodeLister corelister.NodeLister
csiDriverLister storagelister.CSIDriverLister
csiNodeLister storagelister.CSINodeLister
Expand Down Expand Up @@ -95,6 +96,7 @@ func NewVSphereController(
ocpConfigInformer := apiClients.ConfigInformers
configMapInformer := kubeInformers.InformersFor(cloudConfigNamespace).Core().V1().ConfigMaps()
infraInformer := ocpConfigInformer.Config().V1().Infrastructures()
storageInformer := apiClients.OCPOperatorInformers.Operator().V1().Storages()
scInformer := kubeInformers.InformersFor("").Storage().V1().StorageClasses()
csiDriverLister := kubeInformers.InformersFor("").Storage().V1().CSIDrivers().Lister()
csiNodeLister := kubeInformers.InformersFor("").Storage().V1().CSINodes().Lister()
Expand All @@ -120,6 +122,7 @@ func NewVSphereController(
csiConfigManifest: csiConfigManifest,
clusterCSIDriverLister: apiClients.ClusterCSIDriverInformer.Lister(),
infraLister: infraInformer.Lister(),
storageLister: storageInformer.Lister(),
}
c.controllers = []conditionalController{}
c.createCSIDriver()
Expand All @@ -131,6 +134,7 @@ func NewVSphereController(
configMapInformer.Informer(),
apiClients.SecretInformer.Informer(),
infraInformer.Informer(),
storageInformer.Informer(),
scInformer.Informer(),
apiClients.ClusterCSIDriverInformer.Informer(),
).WithSync(c.sync).
Expand Down Expand Up @@ -374,6 +378,7 @@ func (c *VSphereController) getCheckAPIDependency(infra *ocpv1.Infrastructure) c
CSINodeLister: c.csiNodeLister,
CSIDriverLister: c.csiDriverLister,
NodeLister: c.nodeLister,
StorageLister: c.storageLister,
}
return checkerApiClient
}
Expand Down
Loading