Skip to content

Commit

Permalink
adds finalizers (#167)
Browse files Browse the repository at this point in the history
* adds watching for owned resources,
it must prevent bug for #159
in case of kubernetes-controller manager removes object, it will be recreated after sync.
NOTE cluster wide objects cannot be owned by namespaced objects, in this case operator creates orphaned objects,
that cannot be deleted with garbage collection. Need to bind it to related CRD in future.

* fixes vmalert config

* adds finalizers for objects
it must fix #164 #159

* fixes rbac

* Fixes naming and docs

* Fixes incorrect prometheus converter start,
fixes tests

* commeneted test
  • Loading branch information
f41gh7 authored Feb 21, 2021
1 parent 18189d8 commit 20975ec
Show file tree
Hide file tree
Showing 22 changed files with 206 additions and 42 deletions.
27 changes: 26 additions & 1 deletion api/v1beta1/additional.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
package v1beta1

import (
"path"

v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime/schema"
"path"
)

const (
Expand All @@ -14,13 +15,37 @@ const (
reloadPath = "/-/reload"
snapshotCreate = "/snapshot/create"
snapshotDelete = "/snapshot/delete"
// FinalizerName name of our finalizer.
FinalizerName = "apps.victoriametrics.com/finalizer"
)

var (
// GroupVersion is group version used to register these objects
SchemeGroupVersion = schema.GroupVersion{Group: "operator.victoriametrics.com", Version: "v1beta1"}
)

// IsContainsFinalizer check if finalizers is set.
func IsContainsFinalizer(src []string, finalizer string) bool {
for _, s := range src {
if s == finalizer {
return true
}
}
return false
}

// RemoveFinalizer - removes given finalizer from finalizers list.
func RemoveFinalizer(src []string, finalizer string) []string {
dst := src[:0]
for _, s := range src {
if s == finalizer {
continue
}
dst = append(dst, s)
}
return dst
}

// EmbeddedObjectMetadata contains a subset of the fields included in k8s.io/apimachinery/pkg/apis/meta/v1.ObjectMeta
// Only fields which are relevant to embedded resources are included.
type EmbeddedObjectMetadata struct {
Expand Down
4 changes: 4 additions & 0 deletions api/v1beta1/vmagent_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,10 @@ func (cr VMAgent) GetPSPName() string {
return cr.Spec.PodSecurityPolicyName
}

func (cr VMAgent) GetNSName() string {
return cr.GetNamespace()
}

func init() {
SchemeBuilder.Register(&VMAgent{}, &VMAgentList{})
}
4 changes: 4 additions & 0 deletions api/v1beta1/vmalert_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,10 @@ func (cr VMAlert) GetPSPName() string {
return cr.Spec.PodSecurityPolicyName
}

func (cr VMAlert) GetNSName() string {
return cr.GetNamespace()
}

func init() {
SchemeBuilder.Register(&VMAlert{}, &VMAlertList{})
}
4 changes: 4 additions & 0 deletions api/v1beta1/vmalertmanager_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,10 @@ func (cr VMAlertmanager) GetPSPName() string {
return cr.Spec.PodSecurityPolicyName
}

func (cr VMAlertmanager) GetNSName() string {
return cr.GetNamespace()
}

func init() {
SchemeBuilder.Register(&VMAlertmanager{}, &VMAlertmanagerList{})
}
4 changes: 4 additions & 0 deletions api/v1beta1/vmcluster_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -778,3 +778,7 @@ func (cr VMCluster) Labels() map[string]string {
}
return labels
}

func (cr VMCluster) GetNSName() string {
return cr.GetNamespace()
}
4 changes: 4 additions & 0 deletions api/v1beta1/vmsingle_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,10 @@ func (cr VMSingle) GetPSPName() string {
return cr.Spec.PodSecurityPolicyName
}

func (cr VMSingle) GetNSName() string {
return cr.GetNamespace()
}

func init() {
SchemeBuilder.Register(&VMSingle{}, &VMSingleList{})
}
2 changes: 2 additions & 0 deletions config/rbac/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,7 @@ rules:
- patch
- update
- watch
- delete
- apiGroups:
- "policy"
resources:
Expand All @@ -324,6 +325,7 @@ rules:
- update
- use
- watch
- delete
- apiGroups:
- ""
resources:
Expand Down
45 changes: 45 additions & 0 deletions controllers/factory/psp/finalize.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
package psp

import (
"context"

"k8s.io/api/policy/v1beta1"
v1 "k8s.io/api/rbac/v1"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/controller-runtime/pkg/client"
)

// DeletePSPChain - removes psp, cluster role and cluster role binding,
// on finalize request for given CRD
func DeletePSPChain(ctx context.Context, rclient client.Client, crd CRDObject) error {
if err := ensurePSPRemoved(ctx, rclient, crd); err != nil {
return err
}
if err := ensureCRBRemoved(ctx, rclient, crd); err != nil {
return err
}
return ensureCRRemoved(ctx, rclient, crd)
}

func ensurePSPRemoved(ctx context.Context, rclient client.Client, crd CRDObject) error {
return safeDelete(ctx, rclient, &v1beta1.PodSecurityPolicy{ObjectMeta: metav1.ObjectMeta{
Name: crd.GetPSPName()}})
}

func ensureCRRemoved(ctx context.Context, rclient client.Client, crd CRDObject) error {
return safeDelete(ctx, rclient, &v1.ClusterRole{ObjectMeta: metav1.ObjectMeta{Name: crd.PrefixedName()}})
}

func ensureCRBRemoved(ctx context.Context, rclient client.Client, crd CRDObject) error {
return safeDelete(ctx, rclient, &v1.ClusterRoleBinding{ObjectMeta: metav1.ObjectMeta{Name: crd.PrefixedName()}})
}

func safeDelete(ctx context.Context, rclient client.Client, r client.Object) error {
if err := rclient.Delete(ctx, r); err != nil {
if !errors.IsNotFound(err) {
return err
}
}
return nil
}
33 changes: 14 additions & 19 deletions controllers/factory/psp/psp.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,16 @@ import (
"context"
"fmt"

k8stools "github.com/VictoriaMetrics/operator/controllers/factory/k8stools"

"k8s.io/apimachinery/pkg/runtime"

v12 "k8s.io/api/rbac/v1"
"k8s.io/apimachinery/pkg/labels"

"k8s.io/utils/pointer"

"github.com/VictoriaMetrics/operator/controllers/factory/k8stools"
v1 "k8s.io/api/core/v1"
"k8s.io/api/policy/v1beta1"
v12 "k8s.io/api/rbac/v1"
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/types"

v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"k8s.io/utils/pointer"
"sigs.k8s.io/controller-runtime/pkg/client"
)

Expand All @@ -29,7 +24,7 @@ type CRDObject interface {
PrefixedName() string
GetServiceAccountName() string
GetPSPName() string
GetNamespace() string
GetNSName() string
}

// CreateOrUpdateServiceAccountWithPSP - creates psp for api object.
Expand All @@ -54,7 +49,7 @@ func CreateOrUpdateServiceAccountWithPSP(ctx context.Context, cr CRDObject, rcli
func CreateServiceAccountForCRD(ctx context.Context, cr CRDObject, rclient client.Client) error {
newSA := buildSA(cr)
var existSA v1.ServiceAccount
if err := rclient.Get(ctx, types.NamespacedName{Name: cr.GetServiceAccountName(), Namespace: cr.GetNamespace()}, &existSA); err != nil {
if err := rclient.Get(ctx, types.NamespacedName{Name: cr.GetServiceAccountName(), Namespace: cr.GetNSName()}, &existSA); err != nil {
if errors.IsNotFound(err) {
return rclient.Create(ctx, newSA)
}
Expand Down Expand Up @@ -142,7 +137,7 @@ func buildSA(cr CRDObject) *v1.ServiceAccount {
return &v1.ServiceAccount{
ObjectMeta: metav1.ObjectMeta{
Name: cr.GetServiceAccountName(),
Namespace: cr.GetNamespace(),
Namespace: cr.GetNSName(),
Labels: cr.Labels(),
Annotations: cr.Annotations(),
OwnerReferences: cr.AsOwner(),
Expand All @@ -153,7 +148,7 @@ func buildSA(cr CRDObject) *v1.ServiceAccount {
func buildClusterRoleForPSP(cr CRDObject) *v12.ClusterRole {
return &v12.ClusterRole{
ObjectMeta: metav1.ObjectMeta{
Namespace: cr.GetNamespace(),
Namespace: cr.GetNSName(),
Name: cr.PrefixedName(),
Labels: cr.Labels(),
Annotations: cr.Annotations(),
Expand All @@ -173,15 +168,15 @@ func buildClusterRoleBinding(cr CRDObject) *v12.ClusterRoleBinding {
return &v12.ClusterRoleBinding{
ObjectMeta: metav1.ObjectMeta{
Name: cr.PrefixedName(),
Namespace: cr.GetNamespace(),
Namespace: cr.GetNSName(),
Labels: cr.Labels(),
Annotations: cr.Annotations(),
},
Subjects: []v12.Subject{
{
Kind: v12.ServiceAccountKind,
Name: cr.GetServiceAccountName(),
Namespace: cr.GetNamespace(),
Namespace: cr.GetNSName(),
},
},
RoleRef: v12.RoleRef{
Expand All @@ -196,7 +191,7 @@ func BuildPSP(cr CRDObject) *v1beta1.PodSecurityPolicy {
return &v1beta1.PodSecurityPolicy{
ObjectMeta: metav1.ObjectMeta{
Name: cr.GetPSPName(),
Namespace: cr.GetNamespace(),
Namespace: cr.GetNSName(),
Labels: cr.Labels(),
Annotations: cr.Annotations(),
},
Expand Down
3 changes: 1 addition & 2 deletions controllers/factory/vmsingle.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ import (
"path"
"sort"

"k8s.io/apimachinery/pkg/labels"

victoriametricsv1beta1 "github.com/VictoriaMetrics/operator/api/v1beta1"
"github.com/VictoriaMetrics/operator/controllers/factory/k8stools"
"github.com/VictoriaMetrics/operator/controllers/factory/psp"
Expand All @@ -17,6 +15,7 @@ import (
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/intstr"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand Down
4 changes: 4 additions & 0 deletions controllers/vmagent_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,10 @@ func (r *VMAgentReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
// Error reading the object - requeue the request.
return ctrl.Result{}, err
}

if err := handleFinalize(ctx, r.Client, instance); err != nil {
return ctrl.Result{}, err
}
if instance.DeletionTimestamp != nil {
return ctrl.Result{}, nil
}
Expand Down
3 changes: 3 additions & 0 deletions controllers/vmalert_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,9 @@ func (r *VMAlertReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
}
return ctrl.Result{}, err
}
if err := handleFinalize(ctx, r.Client, instance); err != nil {
return ctrl.Result{}, err
}
if instance.DeletionTimestamp != nil {
return ctrl.Result{}, nil
}
Expand Down
36 changes: 36 additions & 0 deletions controllers/vmalertmanager_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ package controllers
import (
"context"

"github.com/VictoriaMetrics/operator/controllers/factory/psp"

"github.com/VictoriaMetrics/operator/controllers/factory"
"github.com/VictoriaMetrics/operator/internal/config"
appsv1 "k8s.io/api/apps/v1"
Expand Down Expand Up @@ -66,6 +68,10 @@ func (r *VMAlertmanagerReconciler) Reconcile(ctx context.Context, req ctrl.Reque
}
return ctrl.Result{}, err
}

if err := handleFinalize(ctx, r.Client, instance); err != nil {
return ctrl.Result{}, err
}
if instance.DeletionTimestamp != nil {
return ctrl.Result{}, nil
}
Expand Down Expand Up @@ -96,3 +102,33 @@ func (r *VMAlertmanagerReconciler) SetupWithManager(mgr ctrl.Manager) error {
Owns(&v1.ServiceAccount{}).
Complete(r)
}

type finalizeObject interface {
psp.CRDObject
client.Object
}

func handleFinalize(ctx context.Context, rclient client.Client, instance finalizeObject) error {
if !instance.GetDeletionTimestamp().IsZero() {
// check finalizers.
if victoriametricsv1beta1.IsContainsFinalizer(instance.GetFinalizers(), victoriametricsv1beta1.FinalizerName) {
if err := psp.DeletePSPChain(ctx, rclient, instance); err != nil {
return err
}
instance.SetFinalizers(victoriametricsv1beta1.RemoveFinalizer(instance.GetFinalizers(), victoriametricsv1beta1.FinalizerName))
if err := rclient.Update(ctx, instance); err != nil {
return err
}
}
return nil
}

// append finalizer if needed
if !victoriametricsv1beta1.IsContainsFinalizer(instance.GetFinalizers(), victoriametricsv1beta1.FinalizerName) {
instance.SetFinalizers(append(instance.GetFinalizers(), victoriametricsv1beta1.FinalizerName))
if err := rclient.Update(ctx, instance); err != nil {
return err
}
}
return nil
}
4 changes: 4 additions & 0 deletions controllers/vmcluster_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,10 @@ func (r *VMClusterReconciler) Reconcile(ctx context.Context, request ctrl.Reques
}
return reconcile.Result{}, err
}

if err := handleFinalize(ctx, r.Client, cluster); err != nil {
return ctrl.Result{}, err
}
if cluster.DeletionTimestamp != nil {
return ctrl.Result{}, nil
}
Expand Down
14 changes: 14 additions & 0 deletions controllers/vmprometheusconverter_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,20 @@ func (c *ConverterController) runInformerWithDiscovery(ctx context.Context, grou
return nil
}

func (c *ConverterController) Start(ctx context.Context) error {
var errG errgroup.Group
log.Info("starting prometheus converter")
c.Run(ctx, &errG)
go func() {
log.Info("waiting for prometheus converter to stop")
err := errG.Wait()
if err != nil {
log.Error(err, "error occured at prometheus converter")
}
}()
return nil
}

// Run - starts vmprometheusconverter with background discovery process for each prometheus api object
func (c *ConverterController) Run(ctx context.Context, group *errgroup.Group) {

Expand Down
3 changes: 3 additions & 0 deletions controllers/vmsingle_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,9 @@ func (r *VMSingleReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
}
return ctrl.Result{}, err
}
if err := handleFinalize(ctx, r.Client, instance); err != nil {
return ctrl.Result{}, err
}
if instance.DeletionTimestamp != nil {
return ctrl.Result{}, nil
}
Expand Down
Loading

0 comments on commit 20975ec

Please sign in to comment.