-
Notifications
You must be signed in to change notification settings - Fork 82
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
Cleanup legacy OLM Subscription & CSVs #1282
Open
dprince
wants to merge
4
commits into
openstack-k8s-operators:main
Choose a base branch
from
dprince:csv_cleanup
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+207
−7
Open
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
ce32449
Cleanup legacy OLM Subscription & CSVs
dprince cf12bea
remove the olm.managed label on legacy installed CRDs
dprince ccb4c8d
Cleanup legacy installPlan and Operator resource
dprince e0d44e1
Add cleanup code for any leftover operator refs
dprince File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,6 +23,7 @@ import ( | |
"path/filepath" | ||
"sort" | ||
"strings" | ||
"time" | ||
|
||
"k8s.io/apimachinery/pkg/runtime" | ||
"k8s.io/client-go/kubernetes" | ||
|
@@ -31,6 +32,8 @@ import ( | |
"sigs.k8s.io/controller-runtime/pkg/handler" | ||
"sigs.k8s.io/controller-runtime/pkg/reconcile" | ||
|
||
"k8s.io/apimachinery/pkg/runtime/schema" | ||
|
||
"github.com/go-logr/logr" | ||
condition "github.com/openstack-k8s-operators/lib-common/modules/common/condition" | ||
"github.com/openstack-k8s-operators/lib-common/modules/common/helper" | ||
|
@@ -41,6 +44,7 @@ import ( | |
appsv1 "k8s.io/api/apps/v1" | ||
apierrors "k8s.io/apimachinery/pkg/api/errors" | ||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
uns "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" | ||
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" | ||
"sigs.k8s.io/controller-runtime/pkg/log" | ||
) | ||
|
@@ -58,7 +62,7 @@ type OpenStackReconciler struct { | |
|
||
// GetLog returns a logger object with a prefix of "controller.name" and aditional controller context fields | ||
func (r *OpenStackReconciler) GetLogger(ctx context.Context) logr.Logger { | ||
return log.FromContext(ctx).WithName("Controllers").WithName("OpenStackControlPlane") | ||
return log.FromContext(ctx).WithName("Controllers").WithName("OpenStackOperator") | ||
} | ||
|
||
var ( | ||
|
@@ -110,6 +114,7 @@ func SetupEnv() { | |
// +kubebuilder:rbac:groups=cert-manager.io,resources=issuers,verbs=get;list;watch;create;update;patch;delete; | ||
// +kubebuilder:rbac:groups=cert-manager.io,resources=certificates,verbs=get;list;watch;create;update;patch;delete; | ||
// +kubebuilder:rbac:groups="monitoring.coreos.com",resources=servicemonitors,verbs=list;get;watch;update;create | ||
// +kubebuilder:rbac:groups=operators.coreos.com,resources=clusterserviceversions;subscriptions;installplans;operators,verbs=get;list;delete; | ||
|
||
// Reconcile is part of the main kubernetes reconciliation loop which aims to | ||
// move the current state of the cluster closer to the desired state. | ||
|
@@ -219,12 +224,10 @@ func (r *OpenStackReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( | |
} | ||
} | ||
|
||
// TODO: cleanup obsolete resources here (remove old CSVs, etc) | ||
/* | ||
if err := r.cleanupObsoleteResources(ctx); err != nil { | ||
return ctrl.Result{}, err | ||
} | ||
*/ | ||
// cleanup obsolete resources here (remove old CSVs, etc) | ||
if err := r.cleanupObsoleteResources(ctx, instance); err != nil { | ||
return ctrl.Result{}, err | ||
} | ||
|
||
if err := r.applyManifests(ctx, instance); err != nil { | ||
instance.Status.Conditions.Set(condition.FalseCondition( | ||
|
@@ -236,6 +239,12 @@ func (r *OpenStackReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( | |
return ctrl.Result{}, err | ||
} | ||
|
||
// now that CRDs have been updated (with old olm.managed references removed) | ||
// we can finally cleanup the old operators | ||
if err := r.postCleanupObsoleteResources(ctx, instance); err != nil { | ||
return ctrl.Result{RequeueAfter: time.Duration(5) * time.Second}, err | ||
} | ||
|
||
// Check if all deployments are running | ||
deploymentsRunning, err := r.countDeployments(ctx, instance) | ||
instance.Status.DeployedOperatorCount = &deploymentsRunning | ||
|
@@ -372,6 +381,182 @@ func (r *OpenStackReconciler) renderAndApply( | |
return nil | ||
} | ||
|
||
func isServiceOperatorResource(name string) bool { | ||
//NOTE: test-operator was deployed as a independant package so it may or may not be installed | ||
//NOTE: depending on how watcher-operator is released for FR2 and then in FR3 it may need to be | ||
// added into this list in the future | ||
serviceOperatorNames := []string{"barbican", "cinder", "designate", "glance", "heat", "horizon", "infra", | ||
"ironic", "keystone", "manila", "mariadb", "neutron", "nova", "octavia", "openstack-baremetal", "ovn", | ||
"placement", "rabbitmq-cluster", "swift", "telemetry", "test"} | ||
|
||
for _, item := range serviceOperatorNames { | ||
if strings.Index(name, item) == 0 { | ||
return true | ||
} | ||
} | ||
return false | ||
} | ||
|
||
// cleanupObsoleteResources - deletes CSVs and subscriptions | ||
func (r *OpenStackReconciler) cleanupObsoleteResources(ctx context.Context, instance *operatorv1beta1.OpenStack) error { | ||
Log := r.GetLogger(ctx) | ||
|
||
csvGVR := schema.GroupVersionResource{ | ||
Group: "operators.coreos.com", | ||
Version: "v1alpha1", | ||
Resource: "clusterserviceversions", | ||
} | ||
|
||
subscriptionGVR := schema.GroupVersionResource{ | ||
Group: "operators.coreos.com", | ||
Version: "v1alpha1", | ||
Resource: "subscriptions", | ||
} | ||
|
||
installPlanGVR := schema.GroupVersionResource{ | ||
Group: "operators.coreos.com", | ||
Version: "v1alpha1", | ||
Resource: "installplans", | ||
} | ||
|
||
csvList := &uns.UnstructuredList{} | ||
csvList.SetGroupVersionKind(csvGVR.GroupVersion().WithKind("ClusterServiceVersion")) | ||
err := r.Client.List(ctx, csvList, &client.ListOptions{Namespace: instance.Namespace}) | ||
if err != nil { | ||
return err | ||
} | ||
for _, csv := range csvList.Items { | ||
Log.Info("Found CSV", "name", csv.GetName()) | ||
if isServiceOperatorResource(csv.GetName()) { | ||
err = r.Client.Delete(ctx, &csv) | ||
if err != nil { | ||
return err | ||
} | ||
Log.Info("CSV deleted successfully", "name", csv.GetName()) | ||
} | ||
} | ||
|
||
subscriptionList := &uns.UnstructuredList{} | ||
subscriptionList.SetGroupVersionKind(subscriptionGVR.GroupVersion().WithKind("Subscription")) | ||
err = r.Client.List(ctx, subscriptionList, &client.ListOptions{Namespace: instance.Namespace}) | ||
if err != nil { | ||
return err | ||
} | ||
for _, subscription := range subscriptionList.Items { | ||
Log.Info("Found Subscription", "name", subscription.GetName()) | ||
if isServiceOperatorResource(subscription.GetName()) { | ||
err = r.Client.Delete(ctx, &subscription) | ||
if err != nil { | ||
return err | ||
} | ||
Log.Info("Subscription deleted successfully", "name", subscription.GetName()) | ||
} | ||
} | ||
|
||
// lookup the installplan which has the clusterServiceVersionNames we removed above | ||
// there will be just a single installPlan that has all of them referenced | ||
installPlanList := &uns.UnstructuredList{} | ||
installPlanList.SetGroupVersionKind(installPlanGVR.GroupVersion().WithKind("InstallPlan")) | ||
|
||
err = r.Client.List(ctx, installPlanList, &client.ListOptions{Namespace: instance.Namespace}) | ||
if err != nil { | ||
return err | ||
} | ||
for _, installPlan := range installPlanList.Items { | ||
Log.Info("Found installPlan", "name", installPlan.GetName()) | ||
// this should have a list containing the CSV names of all the old/legacy service operator CSVs | ||
csvNames, found, err := uns.NestedSlice(installPlan.Object, "spec", "clusterServiceVersionNames") | ||
if err != nil { | ||
return err | ||
} | ||
if found { | ||
// just checking for the first one should be sufficient | ||
if isServiceOperatorResource(csvNames[0].(string)) { | ||
err = r.Client.Delete(ctx, &installPlan) | ||
if err != nil { | ||
return err | ||
} | ||
Log.Info("Installplan deleted successfully", "name", installPlan.GetName()) | ||
} | ||
} | ||
} | ||
|
||
return nil | ||
|
||
} | ||
|
||
// postCleanupObsoleteResources - deletes CSVs for old service operator bundles | ||
func (r *OpenStackReconciler) postCleanupObsoleteResources(ctx context.Context, instance *operatorv1beta1.OpenStack) error { | ||
Log := r.GetLogger(ctx) | ||
|
||
operatorGVR := schema.GroupVersionResource{ | ||
Group: "operators.coreos.com", | ||
Version: "v1", | ||
Resource: "operators", | ||
} | ||
|
||
// finally we can remove operator objects as all the refs have been cleaned up: | ||
// 1) CSVs | ||
// 2) Subscriptions | ||
// 3) CRD olm.managed references removed | ||
// 4) installPlan from old service operators removed | ||
operatorList := &uns.UnstructuredList{} | ||
operatorList.SetGroupVersionKind(operatorGVR.GroupVersion().WithKind("Operator")) | ||
err := r.Client.List(ctx, operatorList, &client.ListOptions{Namespace: instance.Namespace}) | ||
if err != nil { | ||
return err | ||
} | ||
for _, operator := range operatorList.Items { | ||
Log.Info("Found Operator", "name", operator.GetName()) | ||
if isServiceOperatorResource(operator.GetName()) { | ||
|
||
refs, found, err := uns.NestedSlice(operator.Object, "status", "components", "refs") | ||
if err != nil { | ||
return err | ||
} | ||
if found { | ||
|
||
// The horizon-operator.openstack-operators has references to old roles/bindings | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Curious as to why Horizon is specifically referenced here. Is this truly something unique to that operator? |
||
// the code below will delete those references before continuing | ||
for _, ref := range refs { | ||
refData := ref.(map[string]interface{}) | ||
Log.Info("Deleting operator reference", "Reference", ref) | ||
obj := uns.Unstructured{} | ||
obj.SetName(refData["name"].(string)) | ||
obj.SetNamespace(refData["namespace"].(string)) | ||
apiParts := strings.Split(refData["apiVersion"].(string), "/") | ||
objGvk := schema.GroupVersionResource{ | ||
Group: apiParts[0], | ||
Version: apiParts[1], | ||
Resource: refData["kind"].(string), | ||
} | ||
obj.SetGroupVersionKind(objGvk.GroupVersion().WithKind(refData["kind"].(string))) | ||
|
||
// references from CSV's should be removed before this function is called | ||
// but this is a safeguard as we do not want to delete them | ||
if refData["kind"].(string) != "ClusterServiceVersion" { | ||
err = r.Client.Delete(ctx, &obj) | ||
if err != nil { | ||
return err | ||
} | ||
} | ||
} | ||
|
||
return fmt.Errorf("Requeuing/Found references for operator name: %s, refs: %v", operator.GetName(), refs) | ||
} | ||
// no refs found so we should be able to successfully delete the operator | ||
err = r.Client.Delete(ctx, &operator) | ||
if err != nil { | ||
return err | ||
} | ||
Log.Info("Operator deleted successfully", "name", operator.GetName()) | ||
} | ||
} | ||
|
||
return nil | ||
|
||
} | ||
|
||
// SetupWithManager sets up the controller with the Manager. | ||
func (r *OpenStackReconciler) SetupWithManager(mgr ctrl.Manager) error { | ||
|
||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe check for "not found" error here in case the CSV was somehow deleted (in which case it can be ignored and we can just continue)? Probably won't happen, but just a thought.