Skip to content

Commit

Permalink
Do not remove plugin resources on operator uninstall. (#162)
Browse files Browse the repository at this point in the history
When removing the CSV, OLM can remove controller's role or rolebinding
before raising the SIGTERM on the controller, which makes the controller
to fail to delete the plugin resources due to lack of permissions.

This is a workaround for some issues related to plugin's resources when
uninstalling the operator. A better approach for plugin
install/uninstall is yet to implement.

To fully disable/uninstall the operator plugin, it must be removed
manually after the operator has been uninstalled.

If they're not removed and the operator is installed again, the
resources are applied but the error is ignored to let the controller
continue working normally.

Also:
- Minor changes to controller RBAC, to have just one resource per
verb list. Otherwise, the same resource can appear in several verbs list
making it a bit hard to follow.
- Added logging traces to the console plugin resources
creation/removal.
  • Loading branch information
greyerof authored Dec 19, 2024
1 parent d334e76 commit 1e8ef74
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 30 deletions.
10 changes: 2 additions & 8 deletions cmd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,8 +161,8 @@ func main() {
setupLog.Error(err, "unable to create controller", "controller", "CertsuiteRun")
os.Exit(1)
}
consolePluginRemovalDone := make(chan error)
if err := r.HandleConsolePlugin(consolePluginRemovalDone); err != nil {

if err := r.HandleConsolePlugin(); err != nil {
setupLog.Error(err, "error has occurred while handling console plugin")
os.Exit(1)
}
Expand All @@ -188,10 +188,4 @@ func main() {
setupLog.Error(err, "problem running manager")
os.Exit(1)
}

// Wait for openshift's console plugin removal procedure to finish.
if err := <-consolePluginRemovalDone; err != nil {
setupLog.Error(err, "failed to remove openshift console plugin resources")
os.Exit(1)
}
}
24 changes: 19 additions & 5 deletions config/rbac/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -22,20 +22,19 @@ rules:
- ""
resources:
- configMaps
- namespaces
- services
verbs:
- create
- delete
- get
- list
- watch
- apiGroups:
- ""
resources:
- configMaps
- secrets
- namespaces
verbs:
- get
- list
- watch
- apiGroups:
- ""
resources:
Expand All @@ -48,6 +47,21 @@ rules:
- patch
- update
- watch
- apiGroups:
- ""
resources:
- secrets
verbs:
- get
- list
- watch
- apiGroups:
- ""
resources:
- services
verbs:
- create
- delete
- apiGroups:
- apps
resources:
Expand Down
37 changes: 20 additions & 17 deletions internal/controller/certsuiterun_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,8 @@ import (
"context"
"fmt"
"os"
"os/signal"
"path/filepath"
"strconv"
"syscall"
"time"

// appsv1 "k8s.io/api/apps/v1"
Expand All @@ -43,6 +41,7 @@ import (
cnfcertjob "github.com/redhat-best-practices-for-k8s/certsuite-operator/internal/controller/cnf-cert-job"
"github.com/redhat-best-practices-for-k8s/certsuite-operator/internal/controller/definitions"
controllerlogger "github.com/redhat-best-practices-for-k8s/certsuite-operator/internal/controller/logger"
k8serrors "k8s.io/apimachinery/pkg/api/errors"
)

var sideCarImage string
Expand Down Expand Up @@ -73,10 +72,12 @@ const (
// +kubebuilder:rbac:groups=best-practices-for-k8s.openshift.io,namespace=certsuite-operator,resources=certsuiteruns/finalizers,verbs=update

// +kubebuilder:rbac:groups="",namespace=certsuite-operator,resources=pods,verbs=get;list;watch;create;update;patch;delete
// +kubebuilder:rbac:groups="",namespace=certsuite-operator,resources=secrets;configMaps,verbs=get;list;watch
// +kubebuilder:rbac:groups="",namespace=certsuite-operator,resources=namespaces;services;configMaps,verbs=create;delete
// +kubebuilder:rbac:groups="",namespace=certsuite-operator,resources=secrets,verbs=get;list;watch
// +kubebuilder:rbac:groups="",namespace=certsuite-operator,resources=configMaps,verbs=get;list;watch;create;delete
// +kubebuilder:rbac:groups="",namespace=certsuite-operator,resources=services,verbs=create;delete
// +kubebuilder:rbac:groups="",namespace=certsuite-operator,resources=namespaces,verbs=get;list

// +kubebuilder:rbac:groups="console.openshift.io",resources=consoleplugins,verbs=create; delete
// +kubebuilder:rbac:groups="console.openshift.io",resources=consoleplugins,verbs=create;delete
// +kubebuilder:rbac:groups="apps",namespace=certsuite-operator,resources=deployments,verbs=create;get;list;watch;delete

func ignoreUpdatePredicate() predicate.Predicate {
Expand Down Expand Up @@ -368,25 +369,27 @@ func (r *CertsuiteRunReconciler) ApplyOperationOnPluginResources(op func(obj cli
return nil
}

func (r *CertsuiteRunReconciler) HandleConsolePlugin(done chan error) error {
func (r *CertsuiteRunReconciler) HandleConsolePlugin() error {
// Create console plugin resources
err := r.ApplyOperationOnPluginResources(func(obj client.Object) error {
return r.Create(context.Background(), obj)
logger.Info("Creating console plugin resource", "namespace", obj.GetNamespace(), "name", obj.GetName(), "kind", obj.GetObjectKind().GroupVersionKind().Kind)
err := r.Create(context.Background(), obj)
if err != nil {
if k8serrors.IsAlreadyExists(err) {
logger.Info("Openshift Console plugin resource already exists", "namespace", obj.GetNamespace(), "name", obj.GetName(), "kind", obj.GetObjectKind().GroupVersionKind().Kind)
} else {
return err
}
}

return nil
})

if err != nil {
return fmt.Errorf("failed to create plugin, err: %v", err)
}
logger.Info("Operator's console plugin was installed successfully.")

// handle console plugin resources in operator termination
sigs := make(chan os.Signal, 1)
signal.Notify(sigs, syscall.SIGTERM, syscall.SIGINT)
go func() {
<-sigs
done <- r.ApplyOperationOnPluginResources(func(obj client.Object) error {
return r.Delete(context.Background(), obj)
})
}()
logger.Info("Operator's console plugin was installed successfully.")

return nil
}
Expand Down

0 comments on commit 1e8ef74

Please sign in to comment.