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.

Sign up for GitHub

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

✨deploy-image/v1-alpha: add finalizers for the Custom Resource created #2793

Merged
merged 1 commit into from
Jul 8, 2022
+397 −27
Merged

✨deploy-image/v1-alpha: add finalizers for the Custom Resource created #2793

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
20 changes: 20 additions & 0 deletions pkg/plugins/golang/deploy-image/v1alpha1/scaffolds/api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,10 @@ func (s *apiScaffolder) Scaffold() error {
return fmt.Errorf("error updating controller: %v", err)
}

if err := s.updateMainEventRecorder(); err != nil {
return fmt.Errorf("error updating main.go: %v", err)
}

if err := scaffold.Execute(
&samples.CRDSample{Port: s.port},
); err != nil {
Expand All @@ -120,6 +124,18 @@ func (s *apiScaffolder) Scaffold() error {
return nil
}

// TODO: replace this implementation by creating its own MainUpdater
// which will have its own controller template which set the recorder
func (s *apiScaffolder) updateMainEventRecorder() error {
defaultMainPath := "main.go"
err := util.InsertCode(defaultMainPath, `Scheme: mgr.GetScheme(),`,
fmt.Sprintf(recorderTemplate, strings.ToLower(s.resource.Kind)))
if err != nil {
return fmt.Errorf("error scaffolding event recorder while creating the controller in main.go: %v", err)
}
return nil
}

func (s *apiScaffolder) scafffoldControllerWithImage(scaffold *machinery.Scaffold) error {
controller := &controllers.Controller{ControllerRuntimeVersion: golangv3scaffolds.ControllerRuntimeVersion,
Image: s.image,
Expand Down Expand Up @@ -245,3 +261,7 @@ const portTemplate = `
ContainerPort: %s.Spec.ContainerPort,
Name: "%s",
}},`

const recorderTemplate = `
Recorder: mgr.GetEventRecorderFor("%s-controller"),
`
69 changes: 66 additions & 3 deletions ...ugins/golang/deploy-image/v1alpha1/scaffolds/internal/templates/controllers/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,27 +66,33 @@ package {{ if and .MultiGroup .Resource.Group }}{{ .Resource.PackageName }}{{ el
import (
Copy link
Member

Choose a reason for hiding this comment

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

@NikhilSharmaWe,

The new occurs because the recorder is empty and needs to be passed when we create the controller and pass it in the manager ( main.go ):

if err = (&controllers.FoorxobReconciler{
   Client:   mgr.GetClient(),
   Scheme:   mgr.GetScheme(),
   Recorder: mgr.GetEventRecorderFor("mykind-controller"),
}).SetupWithManager(mgr); err != nil {
   setupLog.Error(err, "unable to create controller", "controller", "Foorxob")
   os.Exit(1)
}
//+kubebuilder:scaffold:builder

Also, in the controller, it needs to be changed to be Upper case: Recorder record.EventRecorder
Then, when we call the event use Eventf instead of Event since we want to format the message

See that this PR does not have the e2e check as well. It is missing can you come back that?

appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"

"context"
"time"
"fmt"

"k8s.io/apimachinery/pkg/runtime"
"k8s.io/client-go/tools/record"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"

{{ if not (isEmptyStr .Resource.Path) -}}
{{ .Resource.ImportAlias }} "{{ .Resource.Path }}"
{{- end }}
)

const {{ lower .Resource.Kind }}Finalizer = "{{ .Resource.Group }}.{{ .Resource.Domain }}/finalizer"

camilamacedo86 marked this conversation as resolved.
Show resolved Hide resolved
// {{ .Resource.Kind }}Reconciler reconciles a {{ .Resource.Kind }} object
type {{ .Resource.Kind }}Reconciler struct {
client.Client
Scheme *runtime.Scheme
Recorder record.EventRecorder
}
// The following markers are used to generate the rules permissions on config/rbac using controller-gen
// when the command <make manifests> is executed.
Expand All @@ -95,6 +101,7 @@ type {{ .Resource.Kind }}Reconciler struct {
//+kubebuilder:rbac:groups={{ .Resource.QualifiedGroup }},resources={{ .Resource.Plural }},verbs=get;list;watch;create;update;patch;delete
//+kubebuilder:rbac:groups={{ .Resource.QualifiedGroup }},resources={{ .Resource.Plural }}/status,verbs=get;update;patch
//+kubebuilder:rbac:groups={{ .Resource.QualifiedGroup }},resources={{ .Resource.Plural }}/finalizers,verbs=update
//+kubebuilder:rbac:groups=core,resources=events,verbs=create;patch
//+kubebuilder:rbac:groups=apps,resources=deployments,verbs=get;list;watch;create;update;patch;delete
//+kubebuilder:rbac:groups=core,resources=pods,verbs=get;list;watch

Expand All @@ -119,7 +126,7 @@ func (r *{{ .Resource.Kind }}Reconciler) Reconcile(ctx context.Context, req ctrl
{{ lower .Resource.Kind }} := &{{ .Resource.ImportAlias }}.{{ .Resource.Kind }}{}
err := r.Get(ctx, req.NamespacedName, {{ lower .Resource.Kind }})
if err != nil {
if errors.IsNotFound(err) {
if apierrors.IsNotFound(err) {
// Request object not found, could have been deleted after reconcile request.
// Owned objects are automatically garbage collected. For additional cleanup logic use finalizers.
// Return and don't requeue
Expand All @@ -131,10 +138,52 @@ func (r *{{ .Resource.Kind }}Reconciler) Reconcile(ctx context.Context, req ctrl
return ctrl.Result{}, err
}

// Let's add a finalizer. Then, we can define some operations which should
// occurs before the custom resource to be deleted.
// More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/finalizers/
// NOTE: You should not use finalizer to delete the resources that are
// created in this reconciliation and have the ownerRef set by ctrl.SetControllerReference
// because these will get deleted via k8s api
if !controllerutil.ContainsFinalizer({{ lower .Resource.Kind }}, {{ lower .Resource.Kind }}Finalizer) {
log.Info("Adding Finalizer for {{ .Resource.Kind }}")
controllerutil.AddFinalizer({{ lower .Resource.Kind }}, {{ lower .Resource.Kind }}Finalizer)
camilamacedo86 marked this conversation as resolved.
Show resolved Hide resolved
err = r.Update(ctx, {{ lower .Resource.Kind }})
if err != nil {
return ctrl.Result{}, err
}
}

// Check if the {{ .Resource.Kind }} instance is marked to be deleted, which is
// indicated by the deletion timestamp being set.
is{{ .Resource.Kind }}MarkedToBeDeleted := {{ lower .Resource.Kind }}.GetDeletionTimestamp() != nil
if is{{ .Resource.Kind }}MarkedToBeDeleted {
if controllerutil.ContainsFinalizer({{ lower .Resource.Kind }}, {{ lower .Resource.Kind }}Finalizer) {
// Run finalization logic for memcachedFinalizer. If the
// finalization logic fails, don't remove the finalizer so
// that we can retry during the next reconciliation.
log.Info("Performing Finalizer Operations for {{ .Resource.Kind }} before delete CR")
r.doFinalizerOperationsFor{{ .Resource.Kind }}({{ lower .Resource.Kind }})

// Remove memcachedFinalizer. Once all finalizers have been
// removed, the object will be deleted.
if ok:= controllerutil.RemoveFinalizer({{ lower .Resource.Kind }}, {{ lower .Resource.Kind }}Finalizer); !ok{
if err != nil {
log.Error(err, "Failed to remove finalizer for {{ .Resource.Kind }}")
return ctrl.Result{}, err
}
}
err := r.Update(ctx, {{ lower .Resource.Kind }})
if err != nil {
log.Error(err, "Failed to remove finalizer for {{ .Resource.Kind }}")
}
}
return ctrl.Result{}, nil
}

// Check if the deployment already exists, if not create a new one
found := &appsv1.Deployment{}
err = r.Get(ctx, types.NamespacedName{Name: {{ lower .Resource.Kind }}.Name, Namespace: {{ lower .Resource.Kind }}.Namespace}, found)
if err != nil && errors.IsNotFound(err) {
if err != nil && apierrors.IsNotFound(err) {
// Define a new deployment
dep := r.deploymentFor{{ .Resource.Kind }}({{ lower .Resource.Kind }})
log.Info("Creating a new Deployment", "Deployment.Namespace", dep.Namespace, "Deployment.Name", dep.Name)
Expand Down Expand Up @@ -168,9 +217,23 @@ func (r *{{ .Resource.Kind }}Reconciler) Reconcile(ctx context.Context, req ctrl
// the desired state on the cluster
return ctrl.Result{Requeue: true}, nil
}

return ctrl.Result{}, nil
}

// finalize{{ .Resource.Kind }} will perform the required operations before delete the CR.
func (r *{{ .Resource.Kind }}Reconciler) doFinalizerOperationsFor{{ .Resource.Kind }}(cr *{{ .Resource.ImportAlias }}.{{ .Resource.Kind }}) {
// TODO(user): Add the cleanup steps that the operator
// needs to do before the CR can be deleted. Examples
// of finalizers include performing backups and deleting
// resources that are not owned by this CR, like a PVC.
// The following implementation will raise an event
r.Recorder.Event(cr, "Warning", "Deleting",
fmt.Sprintf("Custom Resource %s is being deleted from the namespace %s",
cr.Name,
cr.Namespace))
}

// deploymentFor{{ .Resource.Kind }} returns a {{ .Resource.Kind }} Deployment object
func (r *{{ .Resource.Kind }}Reconciler) deploymentFor{{ .Resource.Kind }}({{ lower .Resource.Kind }} *{{ .Resource.ImportAlias }}.{{ .Resource.Kind }}) *appsv1.Deployment {
ls := labelsFor{{ .Resource.Kind }}({{ lower .Resource.Kind }}.Name)
Expand Down
17 changes: 17 additions & 0 deletions test/e2e/deployimage/plugin_cluster_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,23 @@ func Run(kbc *utils.TestContext) {
return nil
}
EventuallyWithOffset(1, getMemcachedPodStatus, time.Minute, time.Second).Should(Succeed())

//Testing the finalizer
EventuallyWithOffset(1, func() error {
_, err = kbc.Kubectl.Delete(true, "-f", sampleFilePath)
return err
}, time.Minute, time.Second).Should(Succeed())

EventuallyWithOffset(1, func() error {
events, err := kbc.Kubectl.Get(true, "events", "--field-selector=type=Warning",
"-o", "jsonpath={.items[*].message}",
)
ExpectWithOffset(2, err).NotTo(HaveOccurred())
if !strings.Contains(events, "is being deleted from the namespace") {
return err
}
return nil
}, time.Minute, time.Second).Should(Succeed())
}

func uncommentPodStandards(kbc *utils.TestContext) {
Expand Down
7 changes: 7 additions & 0 deletions testdata/project-v3-with-deploy-image/config/rbac/role.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@ rules:
- patch
- update
- watch
- apiGroups:
- ""
resources:
- events
verbs:
- create
- patch
- apiGroups:
- ""
resources:
Expand Down
71 changes: 67 additions & 4 deletions testdata/project-v3-with-deploy-image/controllers/busybox_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,25 +19,31 @@ package controllers
import (
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"

"context"
"fmt"
"time"

"k8s.io/apimachinery/pkg/runtime"
"k8s.io/client-go/tools/record"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
"sigs.k8s.io/controller-runtime/pkg/log"

examplecomv1alpha1 "sigs.k8s.io/kubebuilder/testdata/project-v3-with-deploy-image/api/v1alpha1"
)

const busyboxFinalizer = "example.com.testproject.org/finalizer"

// BusyboxReconciler reconciles a Busybox object
type BusyboxReconciler struct {
client.Client
Scheme *runtime.Scheme
Scheme *runtime.Scheme
Recorder record.EventRecorder
}

// The following markers are used to generate the rules permissions on config/rbac using controller-gen
Expand All @@ -47,6 +53,7 @@ type BusyboxReconciler struct {
//+kubebuilder:rbac:groups=example.com.testproject.org,resources=busyboxes,verbs=get;list;watch;create;update;patch;delete
//+kubebuilder:rbac:groups=example.com.testproject.org,resources=busyboxes/status,verbs=get;update;patch
//+kubebuilder:rbac:groups=example.com.testproject.org,resources=busyboxes/finalizers,verbs=update
//+kubebuilder:rbac:groups=core,resources=events,verbs=create;patch
//+kubebuilder:rbac:groups=apps,resources=deployments,verbs=get;list;watch;create;update;patch;delete
//+kubebuilder:rbac:groups=core,resources=pods,verbs=get;list;watch

Expand All @@ -71,7 +78,7 @@ func (r *BusyboxReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
busybox := &examplecomv1alpha1.Busybox{}
err := r.Get(ctx, req.NamespacedName, busybox)
if err != nil {
if errors.IsNotFound(err) {
if apierrors.IsNotFound(err) {
// Request object not found, could have been deleted after reconcile request.
// Owned objects are automatically garbage collected. For additional cleanup logic use finalizers.
// Return and don't requeue
Expand All @@ -83,10 +90,52 @@ func (r *BusyboxReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
return ctrl.Result{}, err
}

// Let's add a finalizer. Then, we can define some operations which should
// occurs before the custom resource to be deleted.
// More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/finalizers/
// NOTE: You should not use finalizer to delete the resources that are
// created in this reconciliation and have the ownerRef set by ctrl.SetControllerReference
// because these will get deleted via k8s api
if !controllerutil.ContainsFinalizer(busybox, busyboxFinalizer) {
log.Info("Adding Finalizer for Busybox")
controllerutil.AddFinalizer(busybox, busyboxFinalizer)
err = r.Update(ctx, busybox)
if err != nil {
return ctrl.Result{}, err
}
}

// Check if the Busybox instance is marked to be deleted, which is
// indicated by the deletion timestamp being set.
isBusyboxMarkedToBeDeleted := busybox.GetDeletionTimestamp() != nil
if isBusyboxMarkedToBeDeleted {
if controllerutil.ContainsFinalizer(busybox, busyboxFinalizer) {
// Run finalization logic for memcachedFinalizer. If the
// finalization logic fails, don't remove the finalizer so
// that we can retry during the next reconciliation.
log.Info("Performing Finalizer Operations for Busybox before delete CR")
r.doFinalizerOperationsForBusybox(busybox)

// Remove memcachedFinalizer. Once all finalizers have been
// removed, the object will be deleted.
if ok := controllerutil.RemoveFinalizer(busybox, busyboxFinalizer); !ok {
if err != nil {
log.Error(err, "Failed to remove finalizer for Busybox")
return ctrl.Result{}, err
}
}
err := r.Update(ctx, busybox)
if err != nil {
log.Error(err, "Failed to remove finalizer for Busybox")
}
}
return ctrl.Result{}, nil
}

// Check if the deployment already exists, if not create a new one
found := &appsv1.Deployment{}
err = r.Get(ctx, types.NamespacedName{Name: busybox.Name, Namespace: busybox.Namespace}, found)
if err != nil && errors.IsNotFound(err) {
if err != nil && apierrors.IsNotFound(err) {
// Define a new deployment
dep := r.deploymentForBusybox(busybox)
log.Info("Creating a new Deployment", "Deployment.Namespace", dep.Namespace, "Deployment.Name", dep.Name)
Expand Down Expand Up @@ -120,9 +169,23 @@ func (r *BusyboxReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
// the desired state on the cluster
return ctrl.Result{Requeue: true}, nil
}

return ctrl.Result{}, nil
}

// finalizeBusybox will perform the required operations before delete the CR.
func (r *BusyboxReconciler) doFinalizerOperationsForBusybox(cr *examplecomv1alpha1.Busybox) {
// TODO(user): Add the cleanup steps that the operator
// needs to do before the CR can be deleted. Examples
// of finalizers include performing backups and deleting
// resources that are not owned by this CR, like a PVC.
// The following implementation will raise an event
r.Recorder.Event(cr, "Warning", "Deleting",
fmt.Sprintf("Custom Resource %s is being deleted from the namespace %s",
cr.Name,
cr.Namespace))
}

// deploymentForBusybox returns a Busybox Deployment object
func (r *BusyboxReconciler) deploymentForBusybox(busybox *examplecomv1alpha1.Busybox) *appsv1.Deployment {
ls := labelsForBusybox(busybox.Name)
Expand Down
Loading