Skip to content

Commit

Permalink
[MM-16722] Use github.com/banzaicloud/k8s-objectmatcher for all resou…
Browse files Browse the repository at this point in the history 
…rce reconciliation comparisons (#72)

* minio objectmatcher + some refactors

* revert log removal + typo fix

* unexport create

* unexport crate/update + fixing input for Get + override secret if have invalid data

* minio secret fix create

* update mysql cluster with objectmatcher

* log generic update message + clean unused

* mattermost: update resources with objectmatcher

* minio/instance: update with objectmatcher

* add resource version update

* naming unification

* minio: move logging

* move logging into proper scope for upgrade job

* custom objectMatcher annotation

* Reverting to simple spec and labels update for mysql operator.

There is race interaction between mysql operator and mattermost
operator in case of applying patch to whole resource. We should
only update here what is expected to change otherwise it wrecks mysql operator.
Observed behaviour was for example:
- mysql operator stops responding to any changes in resource,
- database remains in read only mode.

* add comment on mysql operator

* generated fields should be preserved

* remove empty line - merge artifact
  • Loading branch information
@gabrieljackson
Witold Konior authored and gabrieljackson committed Oct 1, 2019
1 parent 5673a2f commit 61eb82c
Show file tree
Hide file tree
Showing 4 changed files with 119 additions and 202 deletions.
59 changes: 34 additions & 25 deletions pkg/controller/clusterinstallation/create_resources.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
"github.com/go-logr/logr"
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
v1beta1 "k8s.io/api/extensions/v1beta1"
"k8s.io/api/extensions/v1beta1"
rbacv1beta1 "k8s.io/api/rbac/v1beta1"
"k8s.io/apimachinery/pkg/api/errors"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand All @@ -16,28 +16,51 @@ import (
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
)

const LastAppliedConfig = "mattermost.com/last-applied"

var DefaultAnnotator = objectMatcher.NewAnnotator(LastAppliedConfig)

// Object combines the interfaces that all Kubernetes objects must implement.
type Object interface {
runtime.Object
v1.Object
}

// createResource creates the provided resource and sets the owner
func (r *ReconcileClusterInstallation) createResource(owner v1.Object, resource Object, reqLogger logr.Logger) error {
// create creates the provided resource and sets the owner
func (r *ReconcileClusterInstallation) create(owner v1.Object, desired Object, reqLogger logr.Logger) error {
// adding the last applied annotation to use the object matcher later
// see: https://github.com/banzaicloud/k8s-objectmatcher
err := objectMatcher.DefaultAnnotator.SetLastAppliedAnnotation(resource)
err := DefaultAnnotator.SetLastAppliedAnnotation(desired)
if err != nil {
reqLogger.Error(err, "Error applying the annotation in the resource")
return err
}
err = r.client.Create(context.TODO(), resource)
err = r.client.Create(context.TODO(), desired)
if err != nil {
reqLogger.Error(err, "Error creating resource")
return err
}
return controllerutil.SetControllerReference(owner, desired, r.scheme)
}

return controllerutil.SetControllerReference(owner, resource, r.scheme)
func (r *ReconcileClusterInstallation) update(current, desired Object, reqLogger logr.Logger) error {
patchResult, err := objectMatcher.NewPatchMaker(DefaultAnnotator).Calculate(current, desired)
if err != nil {
reqLogger.Error(err, "error checking the difference in the resource")
return err
}
if !patchResult.IsEmpty() {
if err := DefaultAnnotator.SetLastAppliedAnnotation(desired); err != nil {
reqLogger.Error(err, "error applying the annotation in the resource")
return err
}
reqLogger.Info("updating resource", "name", desired.GetName(), "namespace", desired.GetNamespace())
// Resource version is required for the update, but need to be set after
// the last applied annotation to avoid unnecessary diffs
desired.SetResourceVersion(current.GetResourceVersion())
return r.client.Update(context.TODO(), desired)
}
return nil
}

func (r *ReconcileClusterInstallation) createServiceAccountIfNotExists(owner v1.Object, serviceAccount *corev1.ServiceAccount, reqLogger logr.Logger) error {
Expand All @@ -46,7 +69,7 @@ func (r *ReconcileClusterInstallation) createServiceAccountIfNotExists(owner v1.
err := r.client.Get(context.TODO(), types.NamespacedName{Name: serviceAccount.Name, Namespace: serviceAccount.Namespace}, foundServiceAccount)
if err != nil && errors.IsNotFound(err) {
reqLogger.Info("Creating service account", "name", serviceAccount.Name)
return r.createResource(owner, serviceAccount, reqLogger)
return r.create(owner, serviceAccount, reqLogger)
} else if err != nil {
reqLogger.Error(err, "Failed to check if service account exists")
return err
Expand All @@ -60,7 +83,7 @@ func (r *ReconcileClusterInstallation) createRoleBindingIfNotExists(owner v1.Obj
err := r.client.Get(context.TODO(), types.NamespacedName{Name: roleBinding.Name, Namespace: roleBinding.Namespace}, foundRoleBinding)
if err != nil && errors.IsNotFound(err) {
reqLogger.Info("Creating role binding", "name", roleBinding.Name)
return r.createResource(owner, roleBinding, reqLogger)
return r.create(owner, roleBinding, reqLogger)
} else if err != nil {
reqLogger.Error(err, "Failed to check if role binding exists")
return err
Expand All @@ -74,7 +97,7 @@ func (r *ReconcileClusterInstallation) createServiceIfNotExists(owner v1.Object,
err := r.client.Get(context.TODO(), types.NamespacedName{Name: service.Name, Namespace: service.Namespace}, foundService)
if err != nil && errors.IsNotFound(err) {
reqLogger.Info("Creating service", "name", service.Name)
return r.createResource(owner, service, reqLogger)
return r.create(owner, service, reqLogger)
} else if err != nil {
reqLogger.Error(err, "Failed to check if service exists")
return err
Expand All @@ -88,7 +111,7 @@ func (r *ReconcileClusterInstallation) createIngressIfNotExists(owner v1.Object,
err := r.client.Get(context.TODO(), types.NamespacedName{Name: ingress.Name, Namespace: ingress.Namespace}, foundIngress)
if err != nil && errors.IsNotFound(err) {
reqLogger.Info("Creating ingress", "name", ingress.Name)
return r.createResource(owner, ingress, reqLogger)
return r.create(owner, ingress, reqLogger)
} else if err != nil {
reqLogger.Error(err, "Failed to check if ingress exists")
return err
Expand All @@ -102,25 +125,11 @@ func (r *ReconcileClusterInstallation) createDeploymentIfNotExists(owner v1.Obje
err := r.client.Get(context.TODO(), types.NamespacedName{Name: deployment.Name, Namespace: deployment.Namespace}, foundDeployment)
if err != nil && errors.IsNotFound(err) {
reqLogger.Info("Creating deployment", "name", deployment.Name)
return r.createResource(owner, deployment, reqLogger)
return r.create(owner, deployment, reqLogger)
} else if err != nil {
reqLogger.Error(err, "Failed to check if deployment exists")
return err
}

return nil
}

func (r *ReconcileClusterInstallation) createSecretIfNotExists(owner v1.Object, secret *corev1.Secret, reqLogger logr.Logger) error {
foundSecret := &corev1.Secret{}
err := r.client.Get(context.TODO(), types.NamespacedName{Name: secret.Name, Namespace: secret.Namespace}, foundSecret)
if err != nil && errors.IsNotFound(err) {
reqLogger.Info("Creating secret", "name", secret.Name)
return r.createResource(owner, secret, reqLogger)
} else if err != nil {
reqLogger.Error(err, "Failed to check if secret exists")
return err
}

return nil
}
124 changes: 22 additions & 102 deletions pkg/controller/clusterinstallation/mattermost.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,13 @@ package clusterinstallation
import (
"context"
"fmt"
"reflect"

objectMatcher "github.com/banzaicloud/k8s-objectmatcher/patch"
"github.com/go-logr/logr"
"github.com/pkg/errors"
appsv1 "k8s.io/api/apps/v1"
batchv1 "k8s.io/api/batch/v1"
corev1 "k8s.io/api/core/v1"
v1beta1 "k8s.io/api/extensions/v1beta1"
"k8s.io/api/extensions/v1beta1"
k8sErrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
Expand Down Expand Up @@ -57,102 +55,37 @@ func (r *ReconcileClusterInstallation) checkMattermost(mattermost *mattermostv1a
}

func (r *ReconcileClusterInstallation) checkMattermostService(mattermost *mattermostv1alpha1.ClusterInstallation, resourceName, selectorName string, reqLogger logr.Logger) error {
service := mattermost.GenerateService(resourceName, selectorName)
desired := mattermost.GenerateService(resourceName, selectorName)

err := r.createServiceIfNotExists(mattermost, service, reqLogger)
err := r.createServiceIfNotExists(mattermost, desired, reqLogger)
if err != nil {
return err
}

foundService := &corev1.Service{}
err = r.client.Get(context.TODO(), types.NamespacedName{Name: service.Name, Namespace: service.Namespace}, foundService)
current := &corev1.Service{}
err = r.client.Get(context.TODO(), types.NamespacedName{Name: desired.Name, Namespace: desired.Namespace}, current)
if err != nil {
return err
}

var update bool

updatedLabels := ensureLabels(service.Labels, foundService.Labels)
if !reflect.DeepEqual(updatedLabels, foundService.Labels) {
reqLogger.Info("Updating mattermost service labels")
foundService.Labels = updatedLabels
update = true
}

if !reflect.DeepEqual(service.Annotations, foundService.Annotations) {
reqLogger.Info("Updating mattermost service annotations")
foundService.Annotations = service.Annotations
update = true
}

// If we are using the loadBalancer the ClusterIp is immutable
// and other fields are created in the first time
if mattermost.Spec.UseServiceLoadBalancer {
service.Spec.ClusterIP = foundService.Spec.ClusterIP
service.Spec.ExternalTrafficPolicy = foundService.Spec.ExternalTrafficPolicy
service.Spec.SessionAffinity = foundService.Spec.SessionAffinity
for _, foundPort := range foundService.Spec.Ports {
for i, servicePort := range service.Spec.Ports {
if foundPort.Name == servicePort.Name {
service.Spec.Ports[i].NodePort = foundPort.NodePort
service.Spec.Ports[i].Protocol = foundPort.Protocol
}
}
}
}
if !reflect.DeepEqual(service.Spec, foundService.Spec) {
reqLogger.Info("Updating mattermost service spec")
foundService.Spec = service.Spec
update = true
}

if update {
return r.client.Update(context.TODO(), foundService)
}

return nil
return r.update(current, desired, reqLogger)
}

func (r *ReconcileClusterInstallation) checkMattermostIngress(mattermost *mattermostv1alpha1.ClusterInstallation, resourceName, ingressName string, ingressAnnotations map[string]string, reqLogger logr.Logger) error {
ingress := mattermost.GenerateIngress(resourceName, ingressName, ingressAnnotations)
desired := mattermost.GenerateIngress(resourceName, ingressName, ingressAnnotations)

err := r.createIngressIfNotExists(mattermost, ingress, reqLogger)
err := r.createIngressIfNotExists(mattermost, desired, reqLogger)
if err != nil {
return err
}

foundIngress := &v1beta1.Ingress{}
err = r.client.Get(context.TODO(), types.NamespacedName{Name: ingress.Name, Namespace: ingress.Namespace}, foundIngress)
current := &v1beta1.Ingress{}
err = r.client.Get(context.TODO(), types.NamespacedName{Name: desired.Name, Namespace: desired.Namespace}, current)
if err != nil {
return err
}

var update bool

updatedLabels := ensureLabels(ingress.Labels, foundIngress.Labels)
if !reflect.DeepEqual(updatedLabels, foundIngress.Labels) {
reqLogger.Info("Updating mattermost ingress labels")
foundIngress.Labels = updatedLabels
update = true
}

if !reflect.DeepEqual(ingress.Annotations, foundIngress.Annotations) {
reqLogger.Info("Updating mattermost ingress annotations")
foundIngress.Annotations = ingress.Annotations
update = true
}

if !reflect.DeepEqual(ingress.Spec, foundIngress.Spec) {
reqLogger.Info("Updating mattermost ingress spec")
foundIngress.Spec = ingress.Spec
update = true
}

if update {
return r.client.Update(context.TODO(), foundIngress)
}

return nil
return r.update(current, desired, reqLogger)
}

func (r *ReconcileClusterInstallation) checkMattermostDeployment(mattermost *mattermostv1alpha1.ClusterInstallation, resourceName, ingressName, imageName string, reqLogger logr.Logger) error {
Expand Down Expand Up @@ -190,20 +123,20 @@ func (r *ReconcileClusterInstallation) checkMattermostDeployment(mattermost *mat
isLicensed = true
}

deployment := mattermost.GenerateDeployment(resourceName, ingressName, imageName, dbData.userName, dbData.userPassword, dbData.dbName, externalDB, isLicensed, minioService)
err = r.createDeploymentIfNotExists(mattermost, deployment, reqLogger)
desired := mattermost.GenerateDeployment(resourceName, ingressName, imageName, dbData.userName, dbData.userPassword, dbData.dbName, externalDB, isLicensed, minioService)
err = r.createDeploymentIfNotExists(mattermost, desired, reqLogger)
if err != nil {
return err
}

foundDeployment := &appsv1.Deployment{}
err = r.client.Get(context.TODO(), types.NamespacedName{Name: deployment.Name, Namespace: deployment.Namespace}, foundDeployment)
current := &appsv1.Deployment{}
err = r.client.Get(context.TODO(), types.NamespacedName{Name: desired.Name, Namespace: desired.Namespace}, current)
if err != nil {
reqLogger.Error(err, "Failed to get mattermost deployment")
return err
}

err = r.updateMattermostDeployment(mattermost, deployment, foundDeployment, imageName, reqLogger)
err = r.updateMattermostDeployment(mattermost, current, desired, imageName, reqLogger)
if err != nil {
reqLogger.Error(err, "Failed to update mattermost deployment")
return err
Expand Down Expand Up @@ -246,13 +179,13 @@ func (r *ReconcileClusterInstallation) launchUpdateJob(mi *mattermostv1alpha1.Cl
// If an update is required then the deployment spec is set to:
// - roll forward version
// - keep active MattermostInstallation available by setting maxUnavailable=N-1
func (r *ReconcileClusterInstallation) updateMattermostDeployment(mi *mattermostv1alpha1.ClusterInstallation, new, original *appsv1.Deployment, imageName string, reqLogger logr.Logger) error {
func (r *ReconcileClusterInstallation) updateMattermostDeployment(mattermost *mattermostv1alpha1.ClusterInstallation, current, desired *appsv1.Deployment, imageName string, reqLogger logr.Logger) error {
var update bool

// Look for mattermost container in pod spec and determine if the image
// needs to be updated.
for _, container := range original.Spec.Template.Spec.Containers {
if container.Name == new.Spec.Template.Spec.Containers[0].Name {
for _, container := range current.Spec.Template.Spec.Containers {
if container.Name == desired.Spec.Template.Spec.Containers[0].Name {
if container.Image != imageName {
reqLogger.Info("Current image is not the same as the requested, will upgrade the Mattermost installation")
update = true
Expand All @@ -268,10 +201,10 @@ func (r *ReconcileClusterInstallation) updateMattermostDeployment(mi *mattermost
// we will return and not upgrade the deployment.
if update {
reqLogger.Info(fmt.Sprintf("Running Mattermost image %s upgrade job check", imageName))
alreadyRunning, err := r.fetchRunningUpdateJob(mi, reqLogger)
alreadyRunning, err := r.fetchRunningUpdateJob(mattermost, reqLogger)
if err != nil && k8sErrors.IsNotFound(err) {
reqLogger.Info("Launching update job")
if err = r.launchUpdateJob(mi, new, imageName, reqLogger); err != nil {
if err = r.launchUpdateJob(mattermost, desired, imageName, reqLogger); err != nil {
return errors.Wrap(err, "Launching update job failed")
}
return errors.New("Began update job")
Expand Down Expand Up @@ -321,20 +254,7 @@ func (r *ReconcileClusterInstallation) updateMattermostDeployment(mi *mattermost
reqLogger.Info("Upgrade image job ran successfully")
}

patchResult, err := objectMatcher.DefaultPatchMaker.Calculate(original, new)
if err != nil {
return errors.Wrap(err, "error checking the difference in the deployment")
}

if !patchResult.IsEmpty() {
err := objectMatcher.DefaultAnnotator.SetLastAppliedAnnotation(new)
if err != nil {
return errors.Wrap(err, "error applying the annotation in the deployment")
}

return r.client.Update(context.TODO(), new)
}
return nil
return r.update(current, desired, reqLogger)
}

func (r *ReconcileClusterInstallation) fetchRunningUpdateJob(mi *mattermostv1alpha1.ClusterInstallation, reqLogger logr.Logger) (*batchv1.Job, error) {
Expand Down
Loading

0 comments on commit 61eb82c

Please sign in to comment.