From 0465ab08eaf175f7f64deaf6b37e64d818b6b216 Mon Sep 17 00:00:00 2001 From: Navid Shakibapour Date: Wed, 18 Mar 2020 17:22:22 -0400 Subject: [PATCH 1/2] Adding OpenShift specific annotations --- .../runtimecomponent_controller.go | 13 ++++-------- pkg/utils/utils.go | 20 ++++++++++++++++++- 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/pkg/controller/runtimecomponent/runtimecomponent_controller.go b/pkg/controller/runtimecomponent/runtimecomponent_controller.go index 34400bea0..1d5217f2d 100644 --- a/pkg/controller/runtimecomponent/runtimecomponent_controller.go +++ b/pkg/controller/runtimecomponent/runtimecomponent_controller.go @@ -334,6 +334,10 @@ func (r *ReconcileRuntimeComponent) Reconcile(request reconcile.Request) (reconc reqLogger.V(1).Info(fmt.Sprintf("%s is not supported on the cluster", applicationsv1beta1.SchemeGroupVersion.String())) } + if r.IsOpenShift() { + instance.Annotations = appstacksutils.MergeMaps(appstacksutils.GetOpenShiftAnnotations(instance), instance.Annotations) + } + currentGen := instance.Generation err = r.GetClient().Update(context.TODO(), instance) if err != nil { @@ -440,9 +444,6 @@ func (r *ReconcileRuntimeComponent) Reconcile(request reconcile.Request) (reconc ksvc := &servingv1alpha1.Service{ObjectMeta: defaultMeta} err = r.CreateOrUpdate(ksvc, instance, func() error { appstacksutils.CustomizeKnativeService(ksvc, instance) - if r.IsOpenShift() { - ksvc.Spec.Template.ObjectMeta.Annotations = appstacksutils.MergeMaps(appstacksutils.GetConnectToAnnotation(instance), ksvc.Spec.Template.ObjectMeta.Annotations) - } return nil }) @@ -510,9 +511,6 @@ func (r *ReconcileRuntimeComponent) Reconcile(request reconcile.Request) (reconc appstacksutils.CustomizeStatefulSet(statefulSet, instance) appstacksutils.CustomizePodSpec(&statefulSet.Spec.Template, instance) appstacksutils.CustomizePersistence(statefulSet, instance) - if r.IsOpenShift() { - statefulSet.Annotations = appstacksutils.MergeMaps(appstacksutils.GetConnectToAnnotation(instance), statefulSet.Annotations) - } return nil }) if err != nil { @@ -541,9 +539,6 @@ func (r *ReconcileRuntimeComponent) Reconcile(request reconcile.Request) (reconc err = r.CreateOrUpdate(deploy, instance, func() error { appstacksutils.CustomizeDeployment(deploy, instance) appstacksutils.CustomizePodSpec(&deploy.Spec.Template, instance) - if r.IsOpenShift() { - deploy.Annotations = appstacksutils.MergeMaps(appstacksutils.GetConnectToAnnotation(instance), deploy.Annotations) - } return nil }) if err != nil { diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index 4b65a8db6..ccb02062d 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -233,7 +233,6 @@ func CustomizePodSpec(pts *corev1.PodTemplateSpec, ba common.BaseComponent) { appContainer.Ports[0].ContainerPort = ba.GetService().GetPort() } - appContainer.Image = ba.GetStatus().GetImageReference() if ba.GetService().GetPortName() != "" { appContainer.Ports[0].Name = ba.GetService().GetPortName() @@ -672,6 +671,7 @@ func GetWatchNamespaces() ([]string, error) { // MergeMaps returns a map containing the union of al the key-value pairs from the input maps. The order of the maps passed into the // func, defines the importance. e.g. if (keyA, value1) is in map1, and (keyA, value2) is in map2, mergeMaps(map1, map2) would contain (keyA, value2). +// If the input map is nil, it is treated as empty map. func MergeMaps(maps ...map[string]string) map[string]string { dest := make(map[string]string) @@ -787,6 +787,24 @@ func GetConnectToAnnotation(ba common.BaseComponent) map[string]string { return anno } +// GetOpenShiftAnnotations returns OpenShift specific annotations +func GetOpenShiftAnnotations(ba common.BaseComponent) map[string]string { + // Mapping between the Open Container Initiative <-> OpenShift annotations + ociToOpenShiftAnnoMap := map[string]string{ + "image.opencontainers.org/source": "app.openshift.io/vcs-uri", + "image.opencontainers.org/revision": "app.openshift.io/vcs-ref", + } + + annos := map[string]string{} + for existingAnno, existingAnnoVal := range ba.GetAnnotations() { + if mappedAnno, ok := ociToOpenShiftAnnoMap[existingAnno]; ok { + annos[mappedAnno] = existingAnnoVal + } + } + + return MergeMaps(annos, GetConnectToAnnotation(ba)) +} + // IsClusterWide returns true if watchNamespaces is set to [""] func IsClusterWide(watchNamespaces []string) bool { return len(watchNamespaces) == 1 && watchNamespaces[0] == "" From 6c57491da3f0b6c811397dd6db6c81a400bba96a Mon Sep 17 00:00:00 2001 From: Navid Shakibapour Date: Wed, 18 Mar 2020 18:54:10 -0400 Subject: [PATCH 2/2] Renaming variables + minor updates --- .../runtimecomponent/runtimecomponent_controller.go | 4 +++- pkg/utils/utils.go | 10 +++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/pkg/controller/runtimecomponent/runtimecomponent_controller.go b/pkg/controller/runtimecomponent/runtimecomponent_controller.go index 1d5217f2d..b0fc38af7 100644 --- a/pkg/controller/runtimecomponent/runtimecomponent_controller.go +++ b/pkg/controller/runtimecomponent/runtimecomponent_controller.go @@ -335,7 +335,9 @@ func (r *ReconcileRuntimeComponent) Reconcile(request reconcile.Request) (reconc } if r.IsOpenShift() { - instance.Annotations = appstacksutils.MergeMaps(appstacksutils.GetOpenShiftAnnotations(instance), instance.Annotations) + // The order of items passed to the MergeMaps matters here! Annotations from GetOpenShiftAnnotations have higher importance. Otherwise, + // it is not possible to override converted annotations. + instance.Annotations = appstacksutils.MergeMaps(instance.Annotations, appstacksutils.GetOpenShiftAnnotations(instance)) } currentGen := instance.Generation diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index ccb02062d..549695637 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -789,16 +789,16 @@ func GetConnectToAnnotation(ba common.BaseComponent) map[string]string { // GetOpenShiftAnnotations returns OpenShift specific annotations func GetOpenShiftAnnotations(ba common.BaseComponent) map[string]string { - // Mapping between the Open Container Initiative <-> OpenShift annotations - ociToOpenShiftAnnoMap := map[string]string{ + // Conversion table between the pseudo Open Container Initiative <-> OpenShift annotations + conversionMap := map[string]string{ "image.opencontainers.org/source": "app.openshift.io/vcs-uri", "image.opencontainers.org/revision": "app.openshift.io/vcs-ref", } annos := map[string]string{} - for existingAnno, existingAnnoVal := range ba.GetAnnotations() { - if mappedAnno, ok := ociToOpenShiftAnnoMap[existingAnno]; ok { - annos[mappedAnno] = existingAnnoVal + for from, to := range conversionMap { + if annoVal, ok := ba.GetAnnotations()[from]; ok { + annos[to] = annoVal } }