WIP: Test apply operator status

go.mod

@@ -8,10 +8,10 @@ require (
  github.com/go-logr/logr v1.4.2 // indirect
  github.com/google/go-cmp v0.6.0
  github.com/google/gofuzz v1.2.0 // indirect
- github.com/openshift/api v0.0.0-20240918014254-07bccfd9266f
+ github.com/openshift/api v0.0.0-20240926031850-46b94866c024
  github.com/openshift/build-machinery-go v0.0.0-20240419090851-af9c868bcf52
- github.com/openshift/client-go v0.0.0-20240528061634-b054aa794d87
- github.com/openshift/library-go v0.0.0-20240715191351-e0aa70d55678
+ github.com/openshift/client-go v0.0.0-20240925210910-aaed17e719c5
+ github.com/openshift/library-go v0.0.0-20240925155829-3c41fd1dea0b
  github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring v0.75.1
  github.com/prometheus-operator/prometheus-operator/pkg/client v0.75.1
  github.com/prometheus/client_golang v1.18.0
@@ -23,7 +23,7 @@ require (
  k8s.io/client-go v0.30.2
  k8s.io/component-base v0.30.2
  k8s.io/klog/v2 v2.130.1
- k8s.io/utils v0.0.0-20240711033017-18e509b52bc8 // indirect
+ k8s.io/utils v0.0.0-20240711033017-18e509b52bc8
  sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect
  sigs.k8s.io/structured-merge-diff/v4 v4.4.1 // indirect
 )

go.sum

@@ -131,14 +131,14 @@ github.com/onsi/ginkgo/v2 v2.17.2 h1:7eMhcy3GimbsA3hEnVKdw/PQM9XN9krpKVXsZdph0/g
 github.com/onsi/ginkgo/v2 v2.17.2/go.mod h1:nP2DPOQoNsQmsVyv5rDA8JkXQoCs6goXIvr/PRJ1eCc=
 github.com/onsi/gomega v1.33.1 h1:dsYjIxxSR755MDmKVsaFQTE22ChNBcuuTWgkUDSubOk=
 github.com/onsi/gomega v1.33.1/go.mod h1:U4R44UsT+9eLIaYRB2a5qajjtQYn0hauxvRm16AVYg0=
-github.com/openshift/api v0.0.0-20240918014254-07bccfd9266f h1:jw869n5Wfttrj56lRolhl7+loudoEvxJt4oa7CbO+QM=
-github.com/openshift/api v0.0.0-20240918014254-07bccfd9266f/go.mod h1:OOh6Qopf21pSzqNVCB5gomomBXb8o5sGKZxG2KNpaXM=
+github.com/openshift/api v0.0.0-20240926031850-46b94866c024 h1:MuDuhmvuhnxBQRdSv2v7Rw1bLb90MTGtDTIt8pRhDTg=
+github.com/openshift/api v0.0.0-20240926031850-46b94866c024/go.mod h1:OOh6Qopf21pSzqNVCB5gomomBXb8o5sGKZxG2KNpaXM=
 github.com/openshift/build-machinery-go v0.0.0-20240419090851-af9c868bcf52 h1:bqBwrXG7sbJUqP1Og1bR8FvVh7qb7CrMgy9saKmOZFs=
 github.com/openshift/build-machinery-go v0.0.0-20240419090851-af9c868bcf52/go.mod h1:b1BuldmJlbA/xYtdZvKi+7j5YGB44qJUJDZ9zwiNCfE=
-github.com/openshift/client-go v0.0.0-20240528061634-b054aa794d87 h1:JtLhaGpSEconE+1IKmIgCOof/Len5ceG6H1pk43yv5U=
-github.com/openshift/client-go v0.0.0-20240528061634-b054aa794d87/go.mod h1:3IPD4U0qyovZS4EFady2kqY32m8lGcbs/Wx+yprg9z8=
-github.com/openshift/library-go v0.0.0-20240715191351-e0aa70d55678 h1:H08EzrqjY63m1jlZ+D4sTy9fSGlHsPwViyxFrWTIh4A=
-github.com/openshift/library-go v0.0.0-20240715191351-e0aa70d55678/go.mod h1:PdASVamWinll2BPxiUpXajTwZxV8A1pQbWEsCN1od7I=
+github.com/openshift/client-go v0.0.0-20240925210910-aaed17e719c5 h1:1yz2alsLpp8L99THG6tWXzeds45hf6jXI4bNoTxEim8=
+github.com/openshift/client-go v0.0.0-20240925210910-aaed17e719c5/go.mod h1:axlsYEU3WeMRlIvHdsSKl/wP17k8oZgHCtPy9WgAMts=
+github.com/openshift/library-go v0.0.0-20240925155829-3c41fd1dea0b h1:NOlXo7ld4xRPBKPxF38PB9m8hdfqljUPcf8bX+7aJPo=
+github.com/openshift/library-go v0.0.0-20240925155829-3c41fd1dea0b/go.mod h1:f8QcnrooSwGa96xI4UaKbKGJZskhTCGeimXKyc4t/ZU=
 github.com/orisano/pixelmatch v0.0.0-20220722002657-fb0b55479cde/go.mod h1:nZgzbfBr3hhjoZnS66nKrHmduYNpc34ny7RK4z5/HM0=
 github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
 github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=

pkg/csoclients/csoclients.go

@@ -5,6 +5,8 @@ import (
  "time"
 
  "github.com/openshift/library-go/pkg/config/client"
+ "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+ "k8s.io/apimachinery/pkg/runtime"
  "k8s.io/client-go/dynamic/dynamicinformer"
  "k8s.io/client-go/rest"
 
@@ -16,20 +18,24 @@ import (
  "k8s.io/client-go/kubernetes"
  "k8s.io/client-go/restmapper"
 
+ operatorv1 "github.com/openshift/api/operator/v1"
  cfgclientset "github.com/openshift/client-go/config/clientset/versioned"
  cfginformers "github.com/openshift/client-go/config/informers/externalversions"
+ applyoperatorv1 "github.com/openshift/client-go/operator/applyconfigurations/operator/v1"
  opclient "github.com/openshift/client-go/operator/clientset/versioned"
  opinformers "github.com/openshift/client-go/operator/informers/externalversions"
- "github.com/openshift/cluster-storage-operator/pkg/operatorclient"
  "github.com/openshift/library-go/pkg/controller/controllercmd"
+ "github.com/openshift/library-go/pkg/operator/genericoperatorclient"
  "github.com/openshift/library-go/pkg/operator/v1helpers"
  prominformer "github.com/prometheus-operator/prometheus-operator/pkg/client/informers/externalversions"
  promclient "github.com/prometheus-operator/prometheus-operator/pkg/client/versioned"
 )
 
 type Clients struct {
  // Client for CSO's CR
- OperatorClient *operatorclient.OperatorClient
+ OperatorClient v1helpers.OperatorClientWithFinalizers
+ OperatorClientInformers dynamicinformer.DynamicSharedInformerFactory
+
  // Kubernetes API client
  KubeClient kubernetes.Interface
  // Kubernetes API informers, per namespace
@@ -127,9 +133,15 @@ func NewClients(controllerConfig *controllercmd.ControllerContext, resync time.D
  }
  c.MonitoringInformer = prominformer.NewSharedInformerFactory(c.MonitoringClient, resync)
 
- c.OperatorClient = &operatorclient.OperatorClient{
- Informers: c.OperatorInformers,
- Client: c.OperatorClientSet,
+ c.OperatorClient, c.OperatorClientInformers, err = genericoperatorclient.NewClusterScopedOperatorClient(
+ controllerConfig.KubeConfig,
+ operatorv1.GroupVersion.WithResource("storages"),
+ operatorv1.GroupVersion.WithKind("Storage"),
+ extractOperatorSpec,
+ extractOperatorStatus,
+ )
+ if err != nil {
+ return nil, err
  }
 
  dc, err := discovery.NewDiscoveryClientForConfig(controllerConfig.KubeConfig)
@@ -231,9 +243,15 @@ func NewHypershiftGuestClients(
  }
  c.MonitoringInformer = prominformer.NewSharedInformerFactory(c.MonitoringClient, resync)
 
- c.OperatorClient = &operatorclient.OperatorClient{
- Informers: c.OperatorInformers,
- Client: c.OperatorClientSet,
+ c.OperatorClient, c.OperatorClientInformers, err = genericoperatorclient.NewClusterScopedOperatorClient(
+ controllerConfig.KubeConfig,
+ operatorv1.GroupVersion.WithResource("storages"),
+ operatorv1.GroupVersion.WithKind("Storage"),
+ extractOperatorSpec,
+ extractOperatorStatus,
+ )
+ if err != nil {
+ return nil, err
  }
 
  dc, err := discovery.NewDiscoveryClientForConfig(kubeRestConfig)
@@ -250,6 +268,7 @@ func StartInformers(clients *Clients, stopCh <-chan struct{}) {
  Start(stopCh <-chan struct{})
  }{
  clients.KubeInformers,
+ clients.OperatorClientInformers,
  clients.OperatorInformers,
  clients.ConfigInformers,
  clients.ExtensionInformer,
@@ -265,6 +284,7 @@ func StartGuestInformers(clients *Clients, stopCh <-chan struct{}) {
  Start(stopCh <-chan struct{})
  }{
  clients.KubeInformers,
+ clients.OperatorClientInformers,
  clients.OperatorInformers,
  clients.ConfigInformers,
  clients.ExtensionInformer,
@@ -280,9 +300,41 @@ func StartMgmtInformers(clients *Clients, stopCh <-chan struct{}) {
  Start(stopCh <-chan struct{})
  }{
  clients.KubeInformers,
+ clients.OperatorClientInformers,
  clients.ConfigInformers,
  clients.DynamicInformer,
  } {
  informer.Start(stopCh)
  }
 }
+
+func extractOperatorSpec(obj *unstructured.Unstructured, fieldManager string) (*applyoperatorv1.OperatorSpecApplyConfiguration, error) {
+ castObj := &operatorv1.Storage{}
+ if err := runtime.DefaultUnstructuredConverter.FromUnstructured(obj.Object, castObj); err != nil {
+ return nil, fmt.Errorf("unable to convert to Storage: %w", err)
+ }
+ ret, err := applyoperatorv1.ExtractStorage(castObj, fieldManager)
+ if err != nil {
+ return nil, fmt.Errorf("unable to extract fields for %q: %w", fieldManager, err)
+ }
+ if ret.Spec == nil {
+ return nil, nil
+ }
+ return &ret.Spec.OperatorSpecApplyConfiguration, nil
+}
+
+func extractOperatorStatus(obj *unstructured.Unstructured, fieldManager string) (*applyoperatorv1.OperatorStatusApplyConfiguration, error) {
+ castObj := &operatorv1.Storage{}
+ if err := runtime.DefaultUnstructuredConverter.FromUnstructured(obj.Object, castObj); err != nil {
+ return nil, fmt.Errorf("unable to convert to Storage: %w", err)
+ }
+ ret, err := applyoperatorv1.ExtractStorageStatus(castObj, fieldManager)
+ if err != nil {
+ return nil, fmt.Errorf("unable to extract fields for %q: %w", fieldManager, err)
+ }
+
+ if ret.Status == nil {
+ return nil, nil
+ }
+ return &ret.Status.OperatorStatusApplyConfiguration, nil
+}

pkg/csoclients/fake.go

@@ -1,6 +1,7 @@
 package csoclients
 
 import (
+ operatorv1 "github.com/openshift/api/operator/v1"
  opv1 "github.com/openshift/api/operator/v1"
  fakeconfig "github.com/openshift/client-go/config/clientset/versioned/fake"
  cfginformers "github.com/openshift/client-go/config/informers/externalversions"
@@ -73,13 +74,16 @@ func NewFakeClients(initialObjects *FakeTestObjects) *Clients {
  categoryExpander := restmapper.NewDiscoveryCategoryExpander(kubeClient.Discovery())
  restMapper := restmapper.NewDeferredDiscoveryRESTMapper(memory.NewMemCacheClient(kubeClient.Discovery()))
 
- opClient := operatorclient.OperatorClient{
- Client: operatorClient,
- Informers: operatorInformerFactory,
- }
+ opClient := v1helpers.NewFakeOperatorClient(
+ &operatorv1.OperatorSpec{
+ ManagementState: operatorv1.Managed,
+ },
+ &operatorv1.OperatorStatus{},
+ nil,
+ )
 
  return &Clients{
- OperatorClient: &opClient,
+ OperatorClient: opClient,
  KubeClient: kubeClient,
  KubeInformers: kubeInformers,
  ExtensionClientSet: apiExtClient,

pkg/operator/csidriveroperator/crcontroller.go

@@ -7,14 +7,14 @@ import (
  "time"
 
  operatorapi "github.com/openshift/api/operator/v1"
+ applyoperatorv1 "github.com/openshift/client-go/operator/applyconfigurations/operator/v1"
  opclient "github.com/openshift/client-go/operator/clientset/versioned"
  oplisters "github.com/openshift/client-go/operator/listers/operator/v1"
  "github.com/openshift/cluster-storage-operator/assets"
  "github.com/openshift/cluster-storage-operator/pkg/csoclients"
  "github.com/openshift/cluster-storage-operator/pkg/operator/csidriveroperator/csioperatorclient"
  "github.com/openshift/library-go/pkg/controller/factory"
  "github.com/openshift/library-go/pkg/operator/events"
- "github.com/openshift/library-go/pkg/operator/resource/resourcemerge"
  "github.com/openshift/library-go/pkg/operator/status"
  "github.com/openshift/library-go/pkg/operator/v1helpers"
  apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -124,19 +124,14 @@ func (c *CSIDriverOperatorCRController) Sync(ctx context.Context, syncCtx factor
  return err
  }
 
- newGeneration := operatorapi.GenerationStatus{
- Group: operatorapi.GroupName,
- Resource: "clustercsidrivers",
- Namespace: cr.Namespace,
- Name: cr.Name,
- LastGeneration: cr.ObjectMeta.Generation,
- }
- updateGenerationFn := func(newStatus *operatorapi.OperatorStatus) error {
- resourcemerge.SetGeneration(&newStatus.Generations, newGeneration)
- return nil
- }
+ newGeneration := applyoperatorv1.GenerationStatus().
+ WithGroup(operatorapi.GroupName).
+ WithResource("clustercsidrivers").
+ WithNamespace(cr.Namespace).
+ WithName(cr.Name).
+ WithLastGeneration(cr.ObjectMeta.Generation)
 
- if err := c.syncConditions(ctx, cr.Status.Conditions, updateGenerationFn); err != nil {
+ if err := c.syncConditions(ctx, cr.Status.Conditions, newGeneration); err != nil {
  errs = append(errs, err)
  }
  return errors.NewAggregate(errs)
@@ -185,62 +180,96 @@ func (c *CSIDriverOperatorCRController) applyClusterCSIDriver(ctx context.Contex
  return existing.DeepCopy(), false, nil
 }
 
-func (c *CSIDriverOperatorCRController) syncConditions(ctx context.Context, conditions []operatorapi.OperatorCondition, updatefn v1helpers.UpdateStatusFunc) error {
- var availableCnd operatorapi.OperatorCondition
+func (c *CSIDriverOperatorCRController) syncConditions(ctx context.Context, conditions []operatorapi.OperatorCondition, newGeneration *applyoperatorv1.GenerationStatusApplyConfiguration) error {
+ // Available condition
+ availableCnd := applyoperatorv1.
+ OperatorCondition().
+ WithType(c.crConditionName(operatorapi.OperatorStatusTypeAvailable))
  disabled, msg := c.hasDisabledCondition(conditions)
  if disabled && c.allowDisabled {
  // The driver can't be running. Mark the operator as Available, but with an extra message.
- availableCnd.Status = operatorapi.ConditionTrue
- availableCnd.Reason = "DriverDisabled"
- availableCnd.Message = fmt.Sprintf("CSI driver for %s is disabled: %s", c.name, msg)
+ availableCnd = availableCnd.
+ WithStatus(operatorapi.ConditionTrue).
+ WithReason("DriverDisabled").
+ WithMessage(fmt.Sprintf("CSI driver for %s is disabled: %s", c.name, msg))
  } else {
  // The driver should be running, copy conditions from the CR
- availableCnd = status.UnionCondition(operatorapi.OperatorStatusTypeAvailable, operatorapi.ConditionTrue, nil, conditions...)
- if availableCnd.Status == operatorapi.ConditionUnknown {
- availableCnd.Status = operatorapi.ConditionFalse
- availableCnd.Reason = "WaitForOperator"
- availableCnd.Message = fmt.Sprintf("Waiting for %s operator to report status", c.name)
+ clusterCSIDriverAvailableCnd := status.UnionCondition(operatorapi.OperatorStatusTypeAvailable, operatorapi.ConditionTrue, nil, conditions...)
+ if clusterCSIDriverAvailableCnd.Status == operatorapi.ConditionUnknown {
+ // If the ClusterCSIDriver's Available condition is Unknown, then ours will be false
+ availableCnd = availableCnd.
+ WithStatus(operatorapi.ConditionFalse).
+ WithReason("WaitForOperator").
+ WithMessage(fmt.Sprintf("Waiting for %s operator to report status", c.name))
+ } else {
+ // Copy the ClusterCSIDriver's Available condition as is
+ availableCnd = availableCnd.
+ WithReason(clusterCSIDriverAvailableCnd.Reason).
+ WithStatus(clusterCSIDriverAvailableCnd.Status).
+ WithMessage(clusterCSIDriverAvailableCnd.Message)
  }
  }
- availableCnd.Type = c.crConditionName(operatorapi.OperatorStatusTypeAvailable)
 
- progressingCnd := status.UnionCondition(operatorapi.OperatorStatusTypeProgressing, operatorapi.ConditionFalse, nil, conditions...)
- progressingCnd.Type = c.crConditionName(operatorapi.OperatorStatusTypeProgressing)
- if progressingCnd.Status == operatorapi.ConditionUnknown {
+ // Progressing condition
+ progressingCnd := applyoperatorv1.OperatorCondition().WithType(c.crConditionName(operatorapi.OperatorStatusTypeProgressing))
+ clusterCSIDriverProgressingCnd := status.UnionCondition(operatorapi.OperatorStatusTypeProgressing, operatorapi.ConditionFalse, nil, conditions...)
+ if clusterCSIDriverProgressingCnd.Status == operatorapi.ConditionUnknown {
  if disabled && c.allowDisabled {
- progressingCnd.Status = operatorapi.ConditionFalse
+ progressingCnd = progressingCnd.WithStatus(operatorapi.ConditionFalse)
  } else {
- progressingCnd.Status = operatorapi.ConditionTrue
- progressingCnd.Reason = "WaitForOperator"
- progressingCnd.Message = fmt.Sprintf("Waiting for %s operator to report status", c.name)
+ progressingCnd = progressingCnd.
+ WithStatus(operatorapi.ConditionTrue).
+ WithReason("WaitForOperator").
+ WithMessage(fmt.Sprintf("Waiting for %s operator to report status", c.name))
  }
+ } else {
+ progressingCnd = progressingCnd.
+ WithReason(clusterCSIDriverProgressingCnd.Reason).
+ WithStatus(clusterCSIDriverProgressingCnd.Status).
+ WithMessage(clusterCSIDriverProgressingCnd.Message)
  }
 
+ // Upgradeable condition
  upgradeableConditionType := c.crConditionName(operatorapi.OperatorStatusTypeUpgradeable)
- upgradeableCond := operatorapi.OperatorCondition{
- Type: upgradeableConditionType,
- Status: operatorapi.ConditionTrue,
- }
+ upgradeableCnd := applyoperatorv1.OperatorCondition().
+ WithType(upgradeableConditionType).
+ WithStatus(operatorapi.ConditionTrue)
 
  if hasCondition(conditions, operatorapi.OperatorStatusTypeUpgradeable) {
- upgradeableCond = status.UnionCondition(operatorapi.OperatorStatusTypeUpgradeable, operatorapi.ConditionTrue, nil, conditions...)
- upgradeableCond.Type = upgradeableConditionType
+ clusterCSIDriverUpgradeableCnd := status.UnionCondition(operatorapi.OperatorStatusTypeUpgradeable, operatorapi.ConditionTrue, nil, conditions...)
+ upgradeableCnd = upgradeableCnd.
+ WithReason(clusterCSIDriverUpgradeableCnd.Reason).
+ WithStatus(clusterCSIDriverUpgradeableCnd.Status).
+ WithMessage(clusterCSIDriverUpgradeableCnd.Message)
  }
 
- degradedCnd := status.UnionCondition(operatorapi.OperatorStatusTypeDegraded, operatorapi.ConditionFalse, nil, conditions...)
- degradedCnd.Type = c.crConditionName(operatorapi.OperatorStatusTypeDegraded)
- if degradedCnd.Status == operatorapi.ConditionUnknown {
- degradedCnd.Status = operatorapi.ConditionFalse
+ // Degraded condition
+ degradedCnd := applyoperatorv1.OperatorCondition().WithType(c.crConditionName(operatorapi.OperatorStatusTypeDegraded))
+ clusterCSIDriverDegradedCnd := status.UnionCondition(operatorapi.OperatorStatusTypeDegraded, operatorapi.ConditionFalse, nil, conditions...)
+ if clusterCSIDriverDegradedCnd.Status == operatorapi.ConditionUnknown {
+ degradedCnd = degradedCnd.WithStatus(operatorapi.ConditionFalse)
+ } else {
+ degradedCnd = degradedCnd.
+ WithStatus(clusterCSIDriverDegradedCnd.Status).
+ WithReason(clusterCSIDriverDegradedCnd.Reason).
+ WithMessage(clusterCSIDriverDegradedCnd.Message)
+
  }
 
- _, _, err := v1helpers.UpdateStatus(ctx, c.operatorClient,
- v1helpers.UpdateConditionFn(availableCnd),
- v1helpers.UpdateConditionFn(progressingCnd),
- v1helpers.UpdateConditionFn(degradedCnd),
- v1helpers.UpdateConditionFn(upgradeableCond),
- updatefn,
+ // Create a partial status with conditions and newGeneration
+ status := applyoperatorv1.OperatorStatus().
+ WithConditions(
+ availableCnd,
+ progressingCnd,
+ degradedCnd,
+ upgradeableCnd).
+ WithGenerations(newGeneration)
+
+ return c.operatorClient.ApplyOperatorStatus(
+ ctx,
+ factory.ControllerFieldManager(c.name, "updateOperatorStatus"),
+ status,
  )
- return err
 }
 
 func (c *CSIDriverOperatorCRController) hasDisabledCondition(conditions []operatorapi.OperatorCondition) (bool, string) {