Skip to content

Commit

Permalink
Merge pull request #564 from jakobmoellerdev/OCPEDGE-810-cleanup
Browse files Browse the repository at this point in the history
OCPEDGE-810: chore: remove pre 4.16 components from cluster on startup
  • Loading branch information
openshift-merge-bot[bot] authored Jan 30, 2024
2 parents 3ea3dd6 + 5ae53f1 commit 382b4ae
Show file tree
Hide file tree
Showing 11 changed files with 228 additions and 6 deletions.
4 changes: 4 additions & 0 deletions bundle/manifests/lvms-operator.clusterserviceversion.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,12 @@ spec:
resources:
- deployments
verbs:
- create
- delete
- get
- list
- patch
- update
- watch
- apiGroups:
- apps
Expand Down
16 changes: 14 additions & 2 deletions catalog/lvms-operator/operator.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -86,13 +86,25 @@ properties:
}
}
operatorframework.io/suggested-namespace: openshift-storage
operatorframework.io/suggested-namespace-template: |-
{
"apiVersion": "v1",
"kind": "Namespace",
"metadata": {
"name": "openshift-storage",
"annotations": {
"workload.openshift.io/allowed": "management"
}
}
}
operators.openshift.io/infrastructure-features: '["csi", "disconnected"]'
operators.operatorframework.io/builder: operator-sdk-v1.25.3
operators.openshift.io/valid-subscription: '["OpenShift Container Platform",
"OpenShift Platform Plus"]'
operators.operatorframework.io/builder: operator-sdk-v1.33.0
operators.operatorframework.io/internal-objects: '["logicalvolumes.topolvm.io",
"lvmvolumegroups.lvm.topolvm.io", "lvmvolumegroupnodestatuses.lvm.topolvm.io"]'
operators.operatorframework.io/project_layout: go.kubebuilder.io/v3
repository: https://github.com/openshift/lvm-operator
target.workload.openshift.io/management: '{"effect": "PreferredDuringScheduling"}'
apiServiceDefinitions: {}
crdDescriptions:
owned:
Expand Down
5 changes: 5 additions & 0 deletions cmd/operator/operator.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (
"github.com/openshift/lvm-operator/internal/controllers/persistent-volume"
"github.com/openshift/lvm-operator/internal/controllers/persistent-volume-claim"
internalCSI "github.com/openshift/lvm-operator/internal/csi"
"github.com/openshift/lvm-operator/internal/migration/microlvms"
"github.com/spf13/cobra"
topolvmcontrollers "github.com/topolvm/topolvm/controllers"
"github.com/topolvm/topolvm/driver"
Expand Down Expand Up @@ -158,6 +159,10 @@ func run(cmd *cobra.Command, _ []string, opts *Options) error {
}
}

if err := microlvms.NewCleanup(setupClient, operatorNamespace).RemovePreMicroLVMSComponents(ctx); err != nil {
return fmt.Errorf("failed to run pre 4.16 MicroLVMS cleanup: %w", err)
}

mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{
Scheme: opts.Scheme,
Metrics: metricsserver.Options{
Expand Down
2 changes: 1 addition & 1 deletion cmd/vgmanager/vgmanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ import (
"github.com/openshift/lvm-operator/internal/controllers/vgmanager/lvmd"
"github.com/openshift/lvm-operator/internal/controllers/vgmanager/wipefs"
internalCSI "github.com/openshift/lvm-operator/internal/csi"
"github.com/openshift/lvm-operator/internal/tagging"
"github.com/openshift/lvm-operator/internal/migration/tagging"
"github.com/spf13/cobra"
topolvmClient "github.com/topolvm/topolvm/client"
"github.com/topolvm/topolvm/controllers"
Expand Down
4 changes: 4 additions & 0 deletions config/rbac/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,12 @@ rules:
resources:
- deployments
verbs:
- create
- delete
- get
- list
- patch
- update
- watch
- apiGroups:
- apps
Expand Down
4 changes: 3 additions & 1 deletion internal/cluster/leaderelection.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import (
// see https://kubernetes.io/docs/reference/labels-annotations-taints/#node-role-kubernetes-io-control-plane
const ControlPlaneIDLabel = "node-role.kubernetes.io/control-plane"

const LeaseName = "1136b8a6.topolvm.io"

type LeaderElectionResolver interface {
Resolve(ctx context.Context) (configv1.LeaderElection, error)
}
Expand All @@ -26,7 +28,7 @@ func NewLeaderElectionResolver(
) (LeaderElectionResolver, error) {
defaultElectionConfig := leaderelection.LeaderElectionDefaulting(configv1.LeaderElection{
Disable: !enableLeaderElection,
}, operatorNamespace, "1136b8a6.topolvm.io")
}, operatorNamespace, LeaseName)

return &nodeLookupSNOLeaderElection{
snoCheck: snoCheck,
Expand Down
2 changes: 1 addition & 1 deletion internal/controllers/lvmcluster/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ func (r *Reconciler) GetLogPassthroughOptions() *logpassthrough.Options {
//+kubebuilder:rbac:groups=lvm.topolvm.io,resources=lvmclusters,verbs=get;list;watch;create;update;patch;delete
//+kubebuilder:rbac:groups=apps,resources=daemonsets,verbs=get;list;watch;create;update;patch;delete
//+kubebuilder:rbac:groups=apps,resources=replicasets,verbs=get
//+kubebuilder:rbac:groups=apps,resources=deployments,verbs=get;list;watch
//+kubebuilder:rbac:groups=apps,resources=deployments,verbs=get;list;watch;create;update;patch;delete
//+kubebuilder:rbac:groups=lvm.topolvm.io,resources=lvmclusters/status,verbs=get;update;patch
//+kubebuilder:rbac:groups=lvm.topolvm.io,resources=lvmclusters/finalizers,verbs=update
//+kubebuilder:rbac:groups=lvm.topolvm.io,resources=lvmvolumegroups,verbs=get;list;watch;create;update;patch;delete
Expand Down
102 changes: 102 additions & 0 deletions internal/migration/microlvms/cleanup.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
package microlvms

import (
"context"
"errors"
"fmt"

"github.com/openshift/lvm-operator/internal/cluster"
appsv1 "k8s.io/api/apps/v1"
coordinationv1 "k8s.io/api/coordination/v1"
k8serrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/utils/ptr"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
"sigs.k8s.io/controller-runtime/pkg/log"
)

const (
TopoLVMLegacyControllerName = "topolvm-controller"
TopoLVMLegacyNodeDaemonSetName = "topolvm-node"
)

type Cleanup struct {
namespace string
client client.Client
}

func NewCleanup(client client.Client, namespace string) *Cleanup {
return &Cleanup{
namespace: namespace,
client: client,
}
}

// RemovePreMicroLVMSComponents is a method of the `Cleanup` struct that performs cleanup tasks for the components
// that ran pre MicroLVMS, e.g. separate controllers or daemonsets.
func (c *Cleanup) RemovePreMicroLVMSComponents(ctx context.Context) error {
objs := []client.Object{
// integrated into lvms operator
&appsv1.Deployment{
ObjectMeta: metav1.ObjectMeta{
Name: TopoLVMLegacyControllerName,
Namespace: c.namespace,
},
},
// integrated into vgmanager
&appsv1.DaemonSet{
ObjectMeta: metav1.ObjectMeta{
Name: TopoLVMLegacyNodeDaemonSetName,
Namespace: c.namespace,
},
},
// replaced by Replace Deployment strategy without Lease
&coordinationv1.Lease{
ObjectMeta: metav1.ObjectMeta{
Name: cluster.LeaseName,
Namespace: c.namespace,
},
},
}

results := make(chan error, len(objs))
deleteImmediatelyIfExistsByIndex := func(i int) {
results <- c.deleteImmediatelyIfExists(ctx, objs[i])
}
for i := range objs {
go deleteImmediatelyIfExistsByIndex(i)
}

var errs []error
for i := 0; i < len(objs); i++ {
if err := <-results; err != nil {
errs = append(errs, err)
}
}

if len(errs) > 0 {
return fmt.Errorf("failed to run pre 4.16 MicroLVMS cleanup: %w", errors.Join(errs...))
}

return nil
}

func (c *Cleanup) deleteImmediatelyIfExists(ctx context.Context, obj client.Object) error {
gvk, _ := apiutil.GVKForObject(obj, c.client.Scheme())
logger := log.FromContext(ctx).WithValues("gvk", gvk.String(),
"name", obj.GetName(), "namespace", obj.GetNamespace())

if err := c.client.Delete(ctx, obj, &client.DeleteOptions{
GracePeriodSeconds: ptr.To(int64(0)),
}); err != nil {
if k8serrors.IsNotFound(err) {
logger.V(1).Info("not found, nothing to delete in cleanup.")
return nil
}
return fmt.Errorf("cleanup delete failed: %w", err)
}

logger.Info("delete successful.")
return nil
}
93 changes: 93 additions & 0 deletions internal/migration/microlvms/cleanup_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
package microlvms

import (
"context"
"errors"
"testing"

"github.com/openshift/lvm-operator/internal/cluster"
appsv1 "k8s.io/api/apps/v1"
coordinationv1 "k8s.io/api/coordination/v1"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/fake"
"sigs.k8s.io/controller-runtime/pkg/client/interceptor"
)

func TestRemovePreMicroLVMSComponents(t *testing.T) {
tests := []struct {
name string
exist bool
wantErr bool
}{
{
name: "objects dont exist anymore (post-migration)",
exist: false,
wantErr: false,
},
{
name: "objects still exist (pre-migration)",
exist: true,
wantErr: false,
},
{
name: "objects exist but delete fails",
exist: true,
wantErr: true,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
namespace := "openshift-storage"
fakeClientBuilder := fake.NewClientBuilder().
WithScheme(setUpScheme()).
WithObjects(setUpObjs(tt.exist, namespace)...)
if tt.wantErr {
fakeClientBuilder.WithInterceptorFuncs(interceptor.Funcs{
Delete: func(ctx context.Context, client client.WithWatch, obj client.Object, opts ...client.DeleteOption) error {
return errors.New("delete failed")
},
})
}
cleanup := NewCleanup(fakeClientBuilder.Build(), namespace)
if err := cleanup.RemovePreMicroLVMSComponents(context.Background()); (err != nil) != tt.wantErr {
t.Errorf("RemovePreMicroLVMSComponents() error = %v, wantErr %v", err, tt.wantErr)
}
})
}
}

func setUpScheme() *runtime.Scheme {
scheme := runtime.NewScheme()
_ = appsv1.AddToScheme(scheme)
_ = coordinationv1.AddToScheme(scheme)
return scheme
}

func setUpObjs(exist bool, namespace string) []client.Object {
if exist {
return nil
}
return []client.Object{
&appsv1.Deployment{
ObjectMeta: v1.ObjectMeta{
Name: TopoLVMLegacyControllerName,
Namespace: namespace,
},
},
&appsv1.DaemonSet{
ObjectMeta: v1.ObjectMeta{
Name: TopoLVMLegacyNodeDaemonSetName,
Namespace: namespace,
},
},
&coordinationv1.Lease{
ObjectMeta: v1.ObjectMeta{
Name: cluster.LeaseName,
Namespace: namespace,
},
},
}
}
File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
"github.com/openshift/lvm-operator/api/v1alpha1"
"github.com/openshift/lvm-operator/internal/controllers/vgmanager/lvm"
lvmmocks "github.com/openshift/lvm-operator/internal/controllers/vgmanager/lvm/mocks"
"github.com/openshift/lvm-operator/internal/tagging"
"github.com/openshift/lvm-operator/internal/migration/tagging"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"

Expand Down

0 comments on commit 382b4ae

Please sign in to comment.