Skip to content

Commit

Permalink
Merge pull request #9 from 3scale-labs/owned-references-limitador
Browse files Browse the repository at this point in the history 
controllers/limitador: use owner refs
  • Loading branch information
@eguzki
eguzki authored Jul 20, 2021
2 parents bfa64bd + 4868802 commit f3c038b
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 127 deletions.
2 changes: 1 addition & 1 deletion Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ ENVTEST_ASSETS_DIR = $(shell pwd)/testbin
test: generate fmt vet manifests
mkdir -p $(ENVTEST_ASSETS_DIR)
test -f $(ENVTEST_ASSETS_DIR)/setup-envtest.sh || curl -sSLo $(ENVTEST_ASSETS_DIR)/setup-envtest.sh https://raw.githubusercontent.com/kubernetes-sigs/controller-runtime/v0.6.3/hack/setup-envtest.sh
source $(ENVTEST_ASSETS_DIR)/setup-envtest.sh; fetch_envtest_tools $(ENVTEST_ASSETS_DIR); setup_envtest_env $(ENVTEST_ASSETS_DIR); go test ./... -coverprofile cover.out
source $(ENVTEST_ASSETS_DIR)/setup-envtest.sh; fetch_envtest_tools $(ENVTEST_ASSETS_DIR); setup_envtest_env $(ENVTEST_ASSETS_DIR); go test ./... -coverprofile cover.out -v

# Build manager binary
manager: generate fmt vet
Expand Down
65 changes: 17 additions & 48 deletions controllers/limitador_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,7 @@ import (
"github.com/go-logr/logr"
v1 "k8s.io/api/apps/v1"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"

Expand All @@ -47,34 +45,32 @@ func (r *LimitadorReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
reqLogger := r.Log.WithValues("limitador", req.NamespacedName)

// Delete Limitador deployment and service if needed
limitadorObj := limitadorv1alpha1.Limitador{}
if err := r.Get(context.TODO(), req.NamespacedName, &limitadorObj); err != nil {
limitadorObj := &limitadorv1alpha1.Limitador{}
if err := r.Get(context.TODO(), req.NamespacedName, limitadorObj); err != nil {
if errors.IsNotFound(err) {
if err = r.ensureLimitadorDeploymentIsDeleted(req.NamespacedName); err != nil {
reqLogger.Error(err, "Failed to delete Limitador deployment.")
return ctrl.Result{}, err
}

if err = r.ensureLimitadorServiceIsDeleted(); err != nil {
reqLogger.Error(err, "Failed to delete Limitador service.")
return ctrl.Result{}, err
}

// The deployment and the service should be deleted automatically
// because they have an owner ref to Limitador
return ctrl.Result{}, nil
} else {
reqLogger.Error(err, "Failed to get Limitador object.")
return ctrl.Result{}, err
}
}

if err := r.ensureLimitadorServiceExists(); err != nil {
return ctrl.Result{}, err
if limitadorObj.GetDeletionTimestamp() != nil { // Marked to be deleted
reqLogger.V(1).Info("marked to be deleted")
return ctrl.Result{}, nil
}

desiredDeployment := limitador.LimitadorDeployment(&limitadorObj)
err := r.ensureLimitadorServiceExists(limitadorObj)
reqLogger.V(1).Info("reconcile service", "error", err)
if err != nil {
return ctrl.Result{}, err
}

if err := r.reconcileDeployment(desiredDeployment); err != nil {
reqLogger.Error(err, "Failed to update Limitador deployment.")
err = r.reconcileDeployment(limitador.LimitadorDeployment(limitadorObj))
reqLogger.V(1).Info("reconcile deployment", "error", err)
if err != nil {
return ctrl.Result{}, err
}

Expand Down Expand Up @@ -121,25 +117,8 @@ func (r *LimitadorReconciler) reconcileDeployment(desiredDeployment *v1.Deployme
}
}

func (r *LimitadorReconciler) ensureLimitadorDeploymentIsDeleted(name types.NamespacedName) error {
currentLimitadorDeployment := v1.Deployment{
ObjectMeta: metav1.ObjectMeta{
Name: name.Name,
Namespace: name.Namespace,
},
}

err := r.Delete(context.TODO(), &currentLimitadorDeployment)

if err != nil && !errors.IsNotFound(err) {
return err
}

return nil
}

func (r *LimitadorReconciler) ensureLimitadorServiceExists() error {
limitadorService := limitador.LimitadorService()
func (r *LimitadorReconciler) ensureLimitadorServiceExists(limitadorObj *limitadorv1alpha1.Limitador) error {
limitadorService := limitador.LimitadorService(limitadorObj)
limitadorServiceKey, _ := client.ObjectKeyFromObject(limitadorService)

err := r.Get(context.TODO(), limitadorServiceKey, limitadorService)
Expand All @@ -153,13 +132,3 @@ func (r *LimitadorReconciler) ensureLimitadorServiceExists() error {

return nil
}

func (r *LimitadorReconciler) ensureLimitadorServiceIsDeleted() error {
err := r.Delete(context.TODO(), limitador.LimitadorService())

if err != nil && !errors.IsNotFound(err) {
return err
}

return nil
}
103 changes: 40 additions & 63 deletions controllers/limitador_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,23 @@ package controllers

import (
"context"
limitadorv1alpha1 "github.com/3scale/limitador-operator/api/v1alpha1"
. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
appsv1 "k8s.io/api/apps/v1"
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/uuid"
"sigs.k8s.io/controller-runtime/pkg/client"
"time"

limitadorv1alpha1 "github.com/3scale/limitador-operator/api/v1alpha1"
"github.com/3scale/limitador-operator/pkg/limitador"
)

var _ = Describe("Limitador controller", func() {
const (
LimitadorName = "limitador-test"
LimitadorNamespace = "default"
LimitadorReplicas = 2
LimitadorImage = "quay.io/3scale/limitador"
Expand All @@ -27,27 +30,37 @@ var _ = Describe("Limitador controller", func() {

replicas := LimitadorReplicas
version := LimitadorVersion
limitador := limitadorv1alpha1.Limitador{
TypeMeta: metav1.TypeMeta{
Kind: "Limitador",
APIVersion: "limitador.3scale.net/v1alpha1",
},
ObjectMeta: metav1.ObjectMeta{
Name: LimitadorName,
Namespace: LimitadorNamespace,
},
Spec: limitadorv1alpha1.LimitadorSpec{
Replicas: &replicas,
Version: &version,
},
newLimitador := func() *limitadorv1alpha1.Limitador {
// The name can't start with a number.
name := "a" + string(uuid.NewUUID())

return &limitadorv1alpha1.Limitador{
TypeMeta: metav1.TypeMeta{
Kind: "Limitador",
APIVersion: "limitador.3scale.net/v1alpha1",
},
ObjectMeta: metav1.ObjectMeta{
Name: name,
Namespace: LimitadorNamespace,
},
Spec: limitadorv1alpha1.LimitadorSpec{
Replicas: &replicas,
Version: &version,
},
}
}

deletePropagationPolicy := client.PropagationPolicy(metav1.DeletePropagationForeground)

Context("Creating a new Limitador object", func() {
var limitadorObj *limitadorv1alpha1.Limitador

BeforeEach(func() {
err := k8sClient.Delete(context.TODO(), limitador.DeepCopy())
limitadorObj = newLimitador()
err := k8sClient.Delete(context.TODO(), limitadorObj, deletePropagationPolicy)
Expect(err == nil || errors.IsNotFound(err))

Expect(k8sClient.Create(context.TODO(), limitador.DeepCopy())).Should(Succeed())
Expect(k8sClient.Create(context.TODO(), limitadorObj)).Should(Succeed())
})

It("Should create a new deployment with the right number of replicas and version", func() {
Expand All @@ -57,7 +70,7 @@ var _ = Describe("Limitador controller", func() {
context.TODO(),
types.NamespacedName{
Namespace: LimitadorNamespace,
Name: LimitadorName,
Name: limitadorObj.Name,
},
&createdLimitadorDeployment)

Expand All @@ -74,65 +87,29 @@ var _ = Describe("Limitador controller", func() {

It("Should create a Limitador service", func() {
createdLimitadorService := v1.Service{}
Eventually(func() bool {
err := k8sClient.Get(
context.TODO(),
types.NamespacedName{
Namespace: "default", // Hardcoded for now
Name: "limitador", // Hardcoded for now
},
&createdLimitadorService)

return err == nil
}, timeout, interval).Should(BeTrue())
})
})

Context("Deleting a Limitador object", func() {
BeforeEach(func() {
err := k8sClient.Create(context.TODO(), limitador.DeepCopy())
Expect(err == nil || errors.IsAlreadyExists(err))

Expect(k8sClient.Delete(context.TODO(), limitador.DeepCopy())).Should(Succeed())
})

It("Should delete the limitador deployment", func() {
createdLimitadorDeployment := appsv1.Deployment{}
Eventually(func() bool {
err := k8sClient.Get(
context.TODO(),
types.NamespacedName{
Namespace: LimitadorNamespace,
Name: LimitadorName,
},
&createdLimitadorDeployment)

return errors.IsNotFound(err)
}, timeout, interval).Should(BeTrue())
})

It("Should delete the limitador service", func() {
createdLimitadorService := v1.Service{}
Eventually(func() bool {
err := k8sClient.Get(
context.TODO(),
types.NamespacedName{
Namespace: "default", // Hardcoded for now
Name: "limitador", // Hardcoded for now
Name: limitador.ServiceName, // Hardcoded for now
},
&createdLimitadorService)

return errors.IsNotFound(err)
return err == nil
}, timeout, interval).Should(BeTrue())
})
})

Context("Updating a limitador object", func() {
var limitadorObj *limitadorv1alpha1.Limitador

BeforeEach(func() {
err := k8sClient.Delete(context.TODO(), limitador.DeepCopy())
limitadorObj = newLimitador()
err := k8sClient.Delete(context.TODO(), limitadorObj, deletePropagationPolicy)
Expect(err == nil || errors.IsNotFound(err))

Expect(k8sClient.Create(context.TODO(), limitador.DeepCopy())).Should(Succeed())
Expect(k8sClient.Create(context.TODO(), limitadorObj)).Should(Succeed())
})

It("Should modify the limitador deployment", func() {
Expand All @@ -142,7 +119,7 @@ var _ = Describe("Limitador controller", func() {
context.TODO(),
types.NamespacedName{
Namespace: LimitadorNamespace,
Name: LimitadorName,
Name: limitadorObj.Name,
},
&updatedLimitador)

Expand All @@ -161,7 +138,7 @@ var _ = Describe("Limitador controller", func() {
context.TODO(),
types.NamespacedName{
Namespace: LimitadorNamespace,
Name: LimitadorName,
Name: limitadorObj.Name,
},
&updatedLimitadorDeployment)

Expand Down
40 changes: 25 additions & 15 deletions pkg/limitador/k8s_objects.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,26 +9,26 @@ import (
)

const (
DefaultVersion = "latest"
DefaultReplicas = 1
ServiceName = "limitador"
ServiceNamespace = "default"
Image = "quay.io/3scale/limitador"
StatusEndpoint = "/status"
ServiceHTTPPort = 8080
ServiceGRPCPort = 8081
DefaultVersion = "latest"
DefaultReplicas = 1
ServiceName = "limitador"
Image = "quay.io/3scale/limitador"
StatusEndpoint = "/status"
ServiceHTTPPort = 8080
ServiceGRPCPort = 8081
)

func LimitadorService() *v1.Service {
func LimitadorService(limitador *limitadorv1alpha1.Limitador) *v1.Service {
return &v1.Service{
TypeMeta: metav1.TypeMeta{
Kind: "Service",
APIVersion: "v1",
},
ObjectMeta: metav1.ObjectMeta{
Name: ServiceName,
Namespace: ServiceNamespace,
Labels: labels(),
Name: ServiceName,
Namespace: limitador.ObjectMeta.Namespace, // TODO: revisit later. For now assume same.
Labels: labels(),
OwnerReferences: []metav1.OwnerReference{ownerRefToLimitador(limitador)},
},
Spec: v1.ServiceSpec{
Ports: []v1.ServicePort{
Expand Down Expand Up @@ -69,9 +69,10 @@ func LimitadorDeployment(limitador *limitadorv1alpha1.Limitador) *appsv1.Deploym
APIVersion: "apps/v1",
},
ObjectMeta: metav1.ObjectMeta{
Name: limitador.ObjectMeta.Name, // TODO: revisit later. For now assume same.
Namespace: limitador.ObjectMeta.Namespace, // TODO: revisit later. For now assume same.
Labels: labels(),
Name: limitador.ObjectMeta.Name, // TODO: revisit later. For now assume same.
Namespace: limitador.ObjectMeta.Namespace, // TODO: revisit later. For now assume same.
Labels: labels(),
OwnerReferences: []metav1.OwnerReference{ownerRefToLimitador(limitador)},
},
Spec: appsv1.DeploymentSpec{
Replicas: &replicas,
Expand Down Expand Up @@ -145,3 +146,12 @@ func LimitadorDeployment(limitador *limitadorv1alpha1.Limitador) *appsv1.Deploym
func labels() map[string]string {
return map[string]string{"app": "limitador"}
}

func ownerRefToLimitador(limitador *limitadorv1alpha1.Limitador) metav1.OwnerReference {
return metav1.OwnerReference{
APIVersion: limitador.APIVersion,
Kind: limitador.Kind,
Name: limitador.Name,
UID: limitador.UID,
}
}

0 comments on commit f3c038b

Please sign in to comment.