Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

new controller for managing upgrade of operator #53

Closed
wants to merge 12 commits into from
Closed
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
3 changes: 3 additions & 0 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ COPY go.mod go.sum ./
# and so that source changes don't invalidate our built layer
COPY vendor/ vendor/

# CRD shims for go-test
COPY shim/ shim/

# Copy the project source
COPY main.go Makefile ./
COPY hack/ hack/
Expand Down
8 changes: 8 additions & 0 deletions api/v1alpha1/storageclient_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,20 @@ type StorageClientSpec struct {
OnboardingTicket string `json:"onboardingTicket"`
}

// OperatorVersion holds current and desired version
type OperatorVersion struct {
CurrentVersion string `json:"currentVersion,omitempty"`
DesiredVersion string `json:"desiredVersion,omitempty"`
}

// StorageClientStatus defines the observed state of StorageClient
type StorageClientStatus struct {
Phase storageClientPhase `json:"phase,omitempty"`

// ConsumerID will hold the identity of this cluster inside the attached provider cluster
ConsumerID string `json:"id,omitempty"`

Operator OperatorVersion `json:"operator,omitempty"`
}

//+kubebuilder:object:root=true
Expand Down
16 changes: 16 additions & 0 deletions api/v1alpha1/zz_generated.deepcopy.go

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

29 changes: 13 additions & 16 deletions bundle/manifests/ocs-client-operator.clusterserviceversion.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,14 @@ spec:
- get
- list
- watch
- apiGroups:
- operators.coreos.com
resources:
- subscriptions
verbs:
- get
- list
- patch
- apiGroups:
- security.openshift.io
resources:
Expand Down Expand Up @@ -608,13 +616,11 @@ spec:
- list
- update
- apiGroups:
- config.openshift.io
- ocs.openshift.io
resources:
- clusterversions
- storageclients/status
verbs:
- get
- list
- watch
- patch
serviceAccountName: ocs-client-operator-status-reporter
deployments:
- label:
Expand Down Expand Up @@ -687,7 +693,8 @@ spec:
selector:
matchLabels:
control-plane: controller-manager
strategy: {}
strategy:
type: Recreate
template:
metadata:
annotations:
Expand Down Expand Up @@ -902,16 +909,6 @@ spec:
verbs:
- create
serviceAccountName: ocs-client-operator-csi-rbd-provisioner-sa
- rules:
- apiGroups:
- operators.coreos.com
resources:
- clusterserviceversions
verbs:
- get
- list
- watch
serviceAccountName: ocs-client-operator-status-reporter
strategy: deployment
installModes:
- supported: true
Expand Down
8 changes: 8 additions & 0 deletions bundle/manifests/ocs.openshift.io_storageclients.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,14 @@ spec:
description: ConsumerID will hold the identity of this cluster inside
the attached provider cluster
type: string
operator:
description: OperatorVersion holds current and desired version
properties:
currentVersion:
type: string
desiredVersion:
type: string
type: object
phase:
type: string
type: object
Expand Down
8 changes: 8 additions & 0 deletions config/crd/bases/ocs.openshift.io_storageclients.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,14 @@ spec:
description: ConsumerID will hold the identity of this cluster inside
the attached provider cluster
type: string
operator:
description: OperatorVersion holds current and desired version
properties:
currentVersion:
type: string
desiredVersion:
type: string
type: object
phase:
type: string
type: object
Expand Down
2 changes: 2 additions & 0 deletions config/manager/manager.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ metadata:
labels:
control-plane: controller-manager
spec:
strategy:
type: Recreate
selector:
matchLabels:
control-plane: controller-manager
Expand Down
2 changes: 0 additions & 2 deletions config/rbac/kustomization.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ resources:
- status-reporter-sa.yaml
- status-reporter-clusterrole.yaml
- status-reporter-clusterrole_binding.yaml
- status-reporter-role.yaml
- status-reporter-role_binding.yaml
# CSI RBAC
- csi_cephfs_plugin_clusterrole.yaml
- csi_cephfs_plugin_clusterrole_binding.yaml
Expand Down
8 changes: 8 additions & 0 deletions config/rbac/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,14 @@ rules:
- get
- list
- watch
- apiGroups:
- operators.coreos.com
resources:
- subscriptions
verbs:
- get
- list
- patch
- apiGroups:
- security.openshift.io
resources:
Expand Down
8 changes: 3 additions & 5 deletions config/rbac/status-reporter-clusterrole.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,8 @@ rules:
- list
- update
- apiGroups:
- config.openshift.io
- ocs.openshift.io
resources:
- clusterversions
- storageclients/status
verbs:
- get
- list
- watch
- patch
15 changes: 0 additions & 15 deletions config/rbac/status-reporter-role.yaml

This file was deleted.

12 changes: 0 additions & 12 deletions config/rbac/status-reporter-role_binding.yaml

This file was deleted.

55 changes: 35 additions & 20 deletions controllers/storageclient_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,13 @@ import (
"encoding/json"
"fmt"
"os"
"strings"
"time"

"github.com/blang/semver/v4"
"github.com/red-hat-storage/ocs-client-operator/api/v1alpha1"
"github.com/red-hat-storage/ocs-client-operator/pkg/utils"

configv1 "github.com/openshift/api/config/v1"
opv1a1 "github.com/operator-framework/api/pkg/operators/v1alpha1"
providerClient "github.com/red-hat-storage/ocs-operator/v4/services/provider/client"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
Expand Down Expand Up @@ -61,8 +60,6 @@ const (
storageClientNameLabel = "ocs.openshift.io/storageclient.name"
storageClientNamespaceLabel = "ocs.openshift.io/storageclient.namespace"
storageClientFinalizer = "storageclient.ocs.openshift.io"

csvPrefix = "ocs-client-operator"
)

// StorageClientReconciler reconciles a StorageClient object
Expand All @@ -74,6 +71,11 @@ type StorageClientReconciler struct {
recorder *utils.EventReporter

OperatorNamespace string
OperatorVersion string

// this can be part of reconciler as after a platform update
// operator will get restarted and fetches latest version
PlatformVersion string
}

// SetupWithManager sets up the controller with the Manager.
Expand Down Expand Up @@ -202,6 +204,10 @@ func (s *StorageClientReconciler) reconcilePhases(instance *v1alpha1.StorageClie
return s.acknowledgeOnboarding(instance, externalClusterClient)
}

if res, err := s.reconcileClientStatus(instance); err != nil {
return res, err
}

if res, err := s.reconcileClientStatusReporterJob(instance); err != nil {
return res, err
}
Expand Down Expand Up @@ -271,22 +277,11 @@ func (s *StorageClientReconciler) onboardConsumer(instance *v1alpha1.StorageClie
return reconcile.Result{}, fmt.Errorf("failed to get the clusterVersion version of the OCP cluster: %v", err)
}

// TODO Have a version file corresponding to the release
csvList := opv1a1.ClusterServiceVersionList{}
if err = s.list(&csvList, client.InNamespace(s.OperatorNamespace)); err != nil {
return reconcile.Result{}, fmt.Errorf("failed to list csv resources in ns: %v, err: %v", s.OperatorNamespace, err)
}
csv := utils.Find(csvList.Items, func(csv *opv1a1.ClusterServiceVersion) bool {
return strings.HasPrefix(csv.Name, csvPrefix)
})
if csv == nil {
return reconcile.Result{}, fmt.Errorf("unable to find csv with prefix %q", csvPrefix)
}
name := fmt.Sprintf("storageconsumer-%s", clusterVersion.Spec.ClusterID)
onboardRequest := providerClient.NewOnboardConsumerRequest().
SetConsumerName(name).
SetOnboardingTicket(instance.Spec.OnboardingTicket).
SetClientOperatorVersion(csv.Spec.Version.String())
SetClientOperatorVersion(s.OperatorVersion)
response, err := externalClusterClient.OnboardConsumer(s.ctx, onboardRequest)
if err != nil {
if st, ok := status.FromError(err); ok {
Expand Down Expand Up @@ -445,6 +440,26 @@ func (s *StorageClientReconciler) delete(obj client.Object) error {
return nil
}

func (s *StorageClientReconciler) reconcileClientStatus(instance *v1alpha1.StorageClient) (reconcile.Result, error) {
currentVersion := instance.Status.Operator.CurrentVersion
operatorVersion, _ := semver.Make(s.OperatorVersion)
if currentVersion == "" {
// TODO(lgangava): should we set Major.Minor.Patch or Major.Minor only
Copy link
Contributor

Choose a reason for hiding this comment

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

Yes (.patch)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is visible to user, desiredVersion will only be M.m and I'm setting currentVersion also as M.m and this TODO is to think of any other reasons that it should be M.m.p which I couldn't find.

instance.Status.Operator.CurrentVersion = fmt.Sprintf("%d.%d", operatorVersion.Major, operatorVersion.Minor)
return reconcile.Result{}, nil
}

instanceVersion, _ := semver.Make(currentVersion)
// operator can be backward compatible to it's operand but it couldn't reconcile the object
// which was initially reconciled by higher versioned operator
if operatorVersion.Major < instanceVersion.Major || operatorVersion.Minor < instanceVersion.Minor {
return reconcile.Result{}, fmt.Errorf("backing off from reconciling StorageClient at higher version %q", currentVersion)
}

instance.Status.Operator.CurrentVersion = fmt.Sprintf("%d.%d", operatorVersion.Major, operatorVersion.Minor)
return reconcile.Result{}, nil
}

func (s *StorageClientReconciler) reconcileClientStatusReporterJob(instance *v1alpha1.StorageClient) (reconcile.Result, error) {
// start the cronJob to ping the provider api server
cronJob := &batchv1.CronJob{}
Expand Down Expand Up @@ -489,6 +504,10 @@ func (s *StorageClientReconciler) reconcileClientStatusReporterJob(instance *v1a
Name: utils.OperatorNamespaceEnvVar,
Value: s.OperatorNamespace,
},
{
Name: utils.PlatformVersionEnvVar,
Value: s.PlatformVersion,
},
},
},
},
Expand All @@ -506,7 +525,3 @@ func (s *StorageClientReconciler) reconcileClientStatusReporterJob(instance *v1a
}
return reconcile.Result{}, nil
}

func (s *StorageClientReconciler) list(obj client.ObjectList, listOptions ...client.ListOption) error {
return s.Client.List(s.ctx, obj, listOptions...)
}
Loading
Loading