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

[MM-16722] Use github.com/banzaicloud/k8s-objectmatcher for all resource reconciliation comparisons #72

Merged
merged 20 commits into from
Oct 1, 2019
Merged

[MM-16722] Use github.com/banzaicloud/k8s-objectmatcher for all resource reconciliation comparisons #72

Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
e204320
minio objectmatcher + some refactors
Aug 27, 2019
63c6a6e
revert log removal + typo fix
Aug 27, 2019
5e8cafc
unexport create
Sep 4, 2019
d9d2c8e
unexport crate/update + fixing input for Get + override secret if hav…
Sep 4, 2019
e567d26
minio secret fix create
Sep 7, 2019
2c3790a
update mysql cluster with objectmatcher
Sep 7, 2019
ac77c2d
log generic update message + clean unused
Sep 7, 2019
ffbdcca
mattermost: update resources with objectmatcher
Sep 7, 2019
59ac132
minio/instance: update with objectmatcher
Sep 7, 2019
783ff83
add resource version update
Sep 7, 2019
988f3db
naming unification
Sep 7, 2019
878712e
minio: move logging
Sep 7, 2019
caa17af
move logging into proper scope for upgrade job
Sep 7, 2019
91cb5ed
custom objectMatcher annotation
Sep 8, 2019
d1f0809
Reverting to simple spec and labels update for mysql operator.
Sep 8, 2019
cab74e9
add comment on mysql operator
Sep 8, 2019
c174ab4
generated fields should be preserved
Sep 8, 2019
16bbd9b
merge master
Sep 19, 2019
bcb4c90
remove empty line - merge artifact
Sep 19, 2019
7d4ac57
merge master
Sep 25, 2019
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
59 changes: 34 additions & 25 deletions pkg/controller/clusterinstallation/create_resources.go
View file
Open in desktop
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
}
125 changes: 23 additions & 102 deletions pkg/controller/clusterinstallation/mattermost.go
View file
Open in desktop
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 @@ -49,102 +47,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 @@ -178,20 +111,20 @@ func (r *ReconcileClusterInstallation) checkMattermostDeployment(mattermost *mat
isLicensed = true
}

deployment := mattermost.GenerateDeployment(resourceName, ingressName, imageName, dbUser, dbPassword, externalDB, isLicensed, minioService)
err = r.createDeploymentIfNotExists(mattermost, deployment, reqLogger)
desired := mattermost.GenerateDeployment(resourceName, ingressName, imageName, dbUser, dbPassword, 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 @@ -234,13 +167,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 @@ -256,10 +189,11 @@ 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 @@ -309,20 +243,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