From 9096e2accb3c9b1215f7ab286c8b5d691925c2bc Mon Sep 17 00:00:00 2001 From: rcernich Date: Wed, 2 Oct 2019 12:51:24 -0600 Subject: [PATCH 1/3] MAISTRA-985 track operator version on custom resources Signed-off-by: rcernich --- .../maistra/v1/servicemeshmemberroll_types.go | 11 +++--- pkg/apis/maistra/v1/status.go | 12 ++++++ pkg/controller/common/util.go | 6 +++ .../servicemesh/controlplane/controller.go | 2 +- .../servicemesh/controlplane/deleter.go | 2 +- .../servicemesh/controlplane/pruner.go | 8 ++-- .../servicemesh/controlplane/reconciler.go | 39 +++++++++---------- .../servicemesh/memberroll/controller.go | 2 +- 8 files changed, 49 insertions(+), 33 deletions(-) diff --git a/pkg/apis/maistra/v1/servicemeshmemberroll_types.go b/pkg/apis/maistra/v1/servicemeshmemberroll_types.go index fabefe1390..ef75c210e6 100644 --- a/pkg/apis/maistra/v1/servicemeshmemberroll_types.go +++ b/pkg/apis/maistra/v1/servicemeshmemberroll_types.go @@ -19,8 +19,8 @@ type ServiceMeshMemberRoll struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` - Spec ServiceMeshMemberRollSpec `json:"spec,omitempty"` - Status ServiceMeshMemberRollStatus `json:"status,omitempty"` + Spec ServiceMeshMemberRollSpec `json:"spec,omitempty"` + Status ServiceMeshMemberRollStatus `json:"status,omitempty"` } // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object @@ -39,7 +39,8 @@ type ServiceMeshMemberRollSpec struct { // ServiceMeshMemberRollStatus contains the state last used to reconcile the list type ServiceMeshMemberRollStatus struct { - ObservedGeneration int64 `json:"observedGeneration,omitempty"` - ServiceMeshGeneration int64 `json:"meshGeneration,omitempty"` - ConfiguredMembers []string `json:"configuredMembers,omitempty"` + ObservedGeneration int64 `json:"observedGeneration,omitempty"` + ServiceMeshGeneration int64 `json:"meshGeneration,omitempty"` + ServiceMeshReconciledVersion string `json:"meshReconciledVersion,omitempty"` + ConfiguredMembers []string `json:"configuredMembers,omitempty"` } diff --git a/pkg/apis/maistra/v1/status.go b/pkg/apis/maistra/v1/status.go index 4d49b4fe65..cd8419a586 100644 --- a/pkg/apis/maistra/v1/status.go +++ b/pkg/apis/maistra/v1/status.go @@ -12,6 +12,7 @@ import ( type StatusType struct { Resource string `json:"resource,omitempty"` ObservedGeneration int64 `json:"observedGeneration,omitempty"` + ReconciledVersion string `json:"reconciledVersion,omitempty"` Conditions []Condition `json:"conditions,omitempty"` } @@ -125,6 +126,17 @@ type Condition struct { LastTransitionTime metav1.Time `json:"lastTransitionTime,omitempty"` } +// GetReconciledVersion returns the reconciled version, or a default for older resources +func (s *StatusType) GetReconciledVersion() string { + if s == nil { + return "0.0.0-0" + } + if s.ReconciledVersion == "" { + return fmt.Sprintf("1.0.0-%d", s.ObservedGeneration) + } + return s.ReconciledVersion +} + // GetCondition removes a condition for the list of conditions func (s *StatusType) GetCondition(conditionType ConditionType) Condition { if s == nil { diff --git a/pkg/controller/common/util.go b/pkg/controller/common/util.go index 2c3811676e..3ddf1e9f25 100644 --- a/pkg/controller/common/util.go +++ b/pkg/controller/common/util.go @@ -8,6 +8,7 @@ import ( "sync" "github.com/go-logr/logr" + "github.com/maistra/istio-operator/pkg/version" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -24,6 +25,11 @@ type ResourceManager struct { OperatorNamespace string } +// ReconciledVersion returns a string encompasing the resource generation and the operator version, e.g. "1.0.0-1" (version 1.0.0, generation 1) +func ReconciledVersion(generation int64) string { + return fmt.Sprintf("%s-%d", version.Info.Version, generation) +} + func IndexOf(l []string, s string) int { for i, elem := range l { if elem == s { diff --git a/pkg/controller/servicemesh/controlplane/controller.go b/pkg/controller/servicemesh/controlplane/controller.go index b10a2624ac..461f2c86df 100644 --- a/pkg/controller/servicemesh/controlplane/controller.go +++ b/pkg/controller/servicemesh/controlplane/controller.go @@ -259,7 +259,7 @@ func (r *ReconcileControlPlane) Reconcile(request reconcile.Request) (reconcile. return reconcile.Result{}, err } - if instance.GetGeneration() == instance.Status.ObservedGeneration && + if common.ReconciledVersion(instance.GetGeneration()) == instance.Status.GetReconciledVersion() && instance.Status.GetCondition(v1.ConditionTypeReconciled).Status == v1.ConditionStatusTrue { // sync readiness state err := reconciler.UpdateReadiness() diff --git a/pkg/controller/servicemesh/controlplane/deleter.go b/pkg/controller/servicemesh/controlplane/deleter.go index 7b1d6246a4..fe9b6594d9 100644 --- a/pkg/controller/servicemesh/controlplane/deleter.go +++ b/pkg/controller/servicemesh/controlplane/deleter.go @@ -8,7 +8,7 @@ import ( func (r *ControlPlaneReconciler) Delete() error { r.Manager.GetRecorder(controllerName).Event(r.Instance, corev1.EventTypeNormal, eventReasonDeleting, "Deleting service mesh") - err := r.prune(-1) + err := r.prune("") if err == nil { r.Manager.GetRecorder(controllerName).Event(r.Instance, corev1.EventTypeNormal, eventReasonDeleted, "Successfully deleted service mesh resources") } else { diff --git a/pkg/controller/servicemesh/controlplane/pruner.go b/pkg/controller/servicemesh/controlplane/pruner.go index b6597aa567..58ca73a096 100644 --- a/pkg/controller/servicemesh/controlplane/pruner.go +++ b/pkg/controller/servicemesh/controlplane/pruner.go @@ -2,12 +2,11 @@ package controlplane import ( "context" - "strconv" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" - "github.com/maistra/istio-operator/pkg/apis/maistra/v1" + v1 "github.com/maistra/istio-operator/pkg/apis/maistra/v1" "github.com/maistra/istio-operator/pkg/controller/common" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -71,7 +70,7 @@ var ( } ) -func (r *ControlPlaneReconciler) prune(generation int64) error { +func (r *ControlPlaneReconciler) prune(generation string) error { allErrors := []error{} err := r.pruneResources(namespacedResources, generation, r.Instance.Namespace) if err != nil { @@ -88,9 +87,8 @@ func (r *ControlPlaneReconciler) prune(generation int64) error { return utilerrors.NewAggregate(allErrors) } -func (r *ControlPlaneReconciler) pruneResources(gvks []schema.GroupVersionKind, generation int64, namespace string) error { +func (r *ControlPlaneReconciler) pruneResources(gvks []schema.GroupVersionKind, instanceGeneration string, namespace string) error { allErrors := []error{} - instanceGeneration := strconv.FormatInt(generation, 10) labelSelector := map[string]string{common.OwnerKey: r.Instance.Namespace} for _, gvk := range gvks { objects := &unstructured.UnstructuredList{} diff --git a/pkg/controller/servicemesh/controlplane/reconciler.go b/pkg/controller/servicemesh/controlplane/reconciler.go index 4b0d2b23eb..e34e89b23c 100644 --- a/pkg/controller/servicemesh/controlplane/reconciler.go +++ b/pkg/controller/servicemesh/controlplane/reconciler.go @@ -5,7 +5,6 @@ import ( "fmt" "io/ioutil" "path" - "strconv" "strings" "github.com/ghodss/yaml" @@ -72,7 +71,7 @@ const ( ) func (r *ControlPlaneReconciler) Reconcile() (result reconcile.Result, err error) { - if r.Status.GetCondition(v1.ConditionTypeReconciled).Status != v1.ConditionStatusFalse { + if r.Status.GetCondition(v1.ConditionTypeReconciled).Status != v1.ConditionStatusFalse { r.initializeReconcileStatus() err := r.PostStatus() return reconcile.Result{}, err // ensure that the new reconcile status is posted immediately. Reconciliation will resume when the status update comes back into the operator @@ -177,7 +176,7 @@ func (r *ControlPlaneReconciler) Reconcile() (result reconcile.Result, err error // initialize common data owner := metav1.NewControllerRef(r.Instance, v1.SchemeGroupVersion.WithKind("ServiceMeshControlPlane")) r.ownerRefs = []metav1.OwnerReference{*owner} - r.meshGeneration = strconv.FormatInt(r.Instance.GetGeneration(), 10) + r.meshGeneration = common.ReconciledVersion(r.Instance.GetGeneration()) // Ensure CRDs are installed if err = bootstrap.InstallCRDs(r.Manager); err != nil { @@ -247,31 +246,31 @@ func (r *ControlPlaneReconciler) Reconcile() (result reconcile.Result, err error } } - // we only need to prune if we've actually installed something - if r.Instance.Generation != 1 { - // delete unseen components - reconciliationMessage = "Pruning obsolete resources" - r.Manager.GetRecorder(controllerName).Event(r.Instance, corev1.EventTypeNormal, eventReasonPruning, reconciliationMessage) - r.Log.Info(reconciliationMessage) - err = r.prune(r.Instance.GetGeneration()) - if err != nil { - reconciliationReason = v1.ConditionReasonReconcileError - reconciliationMessage = "Error pruning obsolete resources" - err = errors.Wrap(err, reconciliationMessage) - return - } + // we still need to prune if this is the first generation, e.g. if the operator was updated during the install, + // it's possible that some resources in the original version may not be present in the new version. + // delete unseen components + reconciliationMessage = "Pruning obsolete resources" + r.Manager.GetRecorder(controllerName).Event(r.Instance, corev1.EventTypeNormal, eventReasonPruning, reconciliationMessage) + r.Log.Info(reconciliationMessage) + err = r.prune(r.meshGeneration) + if err != nil { + reconciliationReason = v1.ConditionReasonReconcileError + reconciliationMessage = "Error pruning obsolete resources" + err = errors.Wrap(err, reconciliationMessage) + return } if r.isUpdating() { reconciliationReason = v1.ConditionReasonUpdateSuccessful - reconciliationMessage = fmt.Sprintf("Successfully updated from generation %d to generation %d", r.Status.ObservedGeneration, r.Instance.GetGeneration()) + reconciliationMessage = fmt.Sprintf("Successfully updated from version %s to version %s", r.Status.GetReconciledVersion(), r.meshGeneration) r.Manager.GetRecorder(controllerName).Event(r.Instance, corev1.EventTypeNormal, eventReasonUpdated, reconciliationMessage) } else { reconciliationReason = v1.ConditionReasonInstallSuccessful - reconciliationMessage = fmt.Sprintf("Successfully installed generation %d", r.Instance.GetGeneration()) + reconciliationMessage = fmt.Sprintf("Successfully installed version %s", r.meshGeneration) r.Manager.GetRecorder(controllerName).Event(r.Instance, corev1.EventTypeNormal, eventReasonInstalled, reconciliationMessage) } r.Status.ObservedGeneration = r.Instance.GetGeneration() + r.Status.ReconciledVersion = r.meshGeneration updateReconcileStatus(&r.Status.StatusType, nil) _, err = r.updateReadinessStatus() // this only updates the local object instance; it doesn't post the status update; postReconciliationStatus (called using defer) actually does that @@ -530,11 +529,11 @@ func (r *ControlPlaneReconciler) initializeReconcileStatus() { var eventReason string var conditionReason v1.ConditionReason if r.isUpdating() { - readyMessage = fmt.Sprintf("Updating mesh from generation %d to generation %d", r.Status.ObservedGeneration, r.Instance.GetGeneration()) + readyMessage = fmt.Sprintf("Updating mesh from generation %s to generation %s", r.Status.GetReconciledVersion(), common.ReconciledVersion(r.Instance.GetGeneration())) eventReason = eventReasonUpdating conditionReason = v1.ConditionReasonSpecUpdated } else { - readyMessage = fmt.Sprintf("Installing mesh generation %d", r.Instance.GetGeneration()) + readyMessage = fmt.Sprintf("Installing mesh generation %s", common.ReconciledVersion(r.Instance.GetGeneration())) eventReason = eventReasonInstalling conditionReason = v1.ConditionReasonResourceCreated diff --git a/pkg/controller/servicemesh/memberroll/controller.go b/pkg/controller/servicemesh/memberroll/controller.go index 05bc519364..d46d00a3f2 100644 --- a/pkg/controller/servicemesh/memberroll/controller.go +++ b/pkg/controller/servicemesh/memberroll/controller.go @@ -377,7 +377,7 @@ func (r *ReconcileMemberList) Reconcile(request reconcile.Request) (reconcile.Re } // we don't update the ServiceMeshGeneration in case the other members need to be updated } - } else if mesh.Status.ObservedGeneration != instance.Status.ServiceMeshGeneration { // service mesh has been updated + } else if mesh.Status.GetReconciledVersion() != instance.Status.ServiceMeshReconciledVersion { // service mesh has been updated reqLogger.Info("Reconciling ServiceMeshMemberRoll namespaces with new generation of ServiceMeshControlPlane") // create reconciler From 23fd2911b6fd42d852e8efdeece99a29d6208cba Mon Sep 17 00:00:00 2001 From: rcernich Date: Fri, 4 Oct 2019 10:49:44 -0600 Subject: [PATCH 2/3] MAISTRA-991 only add maistra-version label to top level resources Signed-off-by: rcernich --- tmp/build/patch-charts.sh | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/tmp/build/patch-charts.sh b/tmp/build/patch-charts.sh index c6b3595f90..8187af9e85 100755 --- a/tmp/build/patch-charts.sh +++ b/tmp/build/patch-charts.sh @@ -137,8 +137,8 @@ function patchTemplates() { # - add a maistra-version label to all objects which have a release label find ${HELM_DIR} -name "*.yaml" -o -name "*.yaml.tpl" | \ - xargs sed -i -e 's/^\(.*\)release:\(.*\)$/\1maistra-version: '${MAISTRA_VERSION}'\ -\1release:\2/' + xargs sed -i -e '/^metadata:/,/^[! ]/ { s/^\(.*\)release:\(.*\)$/\1maistra-version: '${MAISTRA_VERSION}'\ +\1release:\2/ }' # MAISTRA-506 add a maistra-control-plane label for deployment specs find ${HELM_DIR} -name "*.yaml" -o -name "*.yaml.tpl" | xargs grep -Hl 'kind: Deployment' |\ @@ -321,6 +321,30 @@ function removeUnsupportedCharts() { -e '/name:.*certmanager/,+2 d' ${HELM_DIR}/istio/requirements.yaml } +function patchDeploymentSelectors() { + find ${HELM_DIR}/istio/charts -not -path "*/gateways/*" -type f |xargs grep -l "^kind:.* Deployment"| xargs grep -L "^ *selector:" | xargs \ + sed -i -e '/^metadata:/,/^template:/ { + /^ *app:.*$/ H + /^ *istio:.*$/ H + /^ *release:.*$/ H + /template:/ { + x + s/\(\n\)/\1 /g + s/\n/\ +\ selector:\ +\ matchLabels:\ +/ + G + } + }' + + sed -i -e '/template:/ i\ +\ selector:\ +\ matchLabels:\ +\ {{- range $key, $val := $spec.labels }}\ +\ {{ $key }}: {{ $val }}\ +\ {{- end }}' ${HELM_DIR}/istio/charts/gateways/templates/deployment.yaml +} copyOverlay @@ -328,6 +352,7 @@ removeUnsupportedCharts patchTemplates patchKialiTemplate patchKialiOpenShift +patchDeploymentSelectors patchMultiTenant source ${SOURCE_DIR}/tmp/build/patch-grafana.sh From 3909b0a215bbc182cc8dd099419a99585d63e9d3 Mon Sep 17 00:00:00 2001 From: rcernich Date: Fri, 4 Oct 2019 10:56:46 -0600 Subject: [PATCH 3/3] MAISTRA-992 use delete/recreate when patching fails during resource updates Signed-off-by: rcernich --- pkg/controller/common/manifestprocessing.go | 17 +++++++++++++++++ pkg/controller/common/patch.go | 5 ++++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/pkg/controller/common/manifestprocessing.go b/pkg/controller/common/manifestprocessing.go index da89e87c2f..683ee56e18 100644 --- a/pkg/controller/common/manifestprocessing.go +++ b/pkg/controller/common/manifestprocessing.go @@ -7,6 +7,7 @@ import ( "github.com/ghodss/yaml" "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" utilerrors "k8s.io/apimachinery/pkg/util/errors" @@ -144,6 +145,22 @@ func (p *ManifestProcessor) processObject(obj *unstructured.Unstructured, compon } else if patch, err = p.PatchFactory.CreatePatch(receiver, obj); err == nil && patch != nil { p.Log.Info("updating existing resource") _, err = patch.Apply() + if errors.IsInvalid(err) { + // patch was invalid, try delete/create + p.Log.Info("patch failed. attempting to delete and recreate the resource") + if deleteErr := p.Client.Delete(context.TODO(), obj, client.PropagationPolicy(metav1.DeletePropagationBackground)); deleteErr == nil { + // we need to remove the resource version, which was updated by the patching process + obj.SetResourceVersion("") + if createErr := p.Client.Create(context.TODO(), obj); createErr == nil { + p.Log.Info("successfully recreated resource after patch failure") + err = nil + } else { + p.Log.Error(createErr, "error trying to recreate resource after patch failure") + } + } else { + p.Log.Error(deleteErr, "error deleting resource for recreation") + } + } } p.Log.V(2).Info("resource reconciliation complete") if err != nil { diff --git a/pkg/controller/common/patch.go b/pkg/controller/common/patch.go index 4f517b5b75..8abf56e323 100644 --- a/pkg/controller/common/patch.go +++ b/pkg/controller/common/patch.go @@ -83,7 +83,10 @@ func (p *PatchFactory) CreatePatch(current, new runtime.Object) (Patch, error) { mergepatch.RequireKeyUnchanged("apiVersion"), mergepatch.RequireKeyUnchanged("kind"), mergepatch.RequireMetadataKeyUnchanged("name"), - mergepatch.RequireMetadataKeyUnchanged("namespace"), + } + // prevent precondition errors for cluster scoped resources + if currentAccessor.GetNamespace() != "" { + preconditions = append(preconditions, mergepatch.RequireMetadataKeyUnchanged("namespace")) } patchBytes, err := jsonmergepatch.CreateThreeWayJSONMergePatch(originalBytes, newBytes, currentBytes, preconditions...) if err != nil {