Fix race of doing an override of annotations that belongs to third co…

…ntrollers (#2682)

provision/kubernetes/provisioner.go

@@ -12,6 +12,7 @@ import (
 	"io"
 	"net/url"
 	"os"
+	"reflect"
 	"sort"
 	"strconv"
 	"strings"
@@ -1348,41 +1349,38 @@ func (p *kubernetesProvisioner) Shutdown(ctx context.Context) error {
 }
 
 func ensureAppCustomResourceSynced(ctx context.Context, client *ClusterClient, a provision.App) error {
-	_, err := loadAndEnsureAppCustomResourceSynced(ctx, client, a)
-	return err
-}
-
-func loadAndEnsureAppCustomResourceSynced(ctx context.Context, client *ClusterClient, a provision.App) (*tsuruv1.App, error) {
 	err := ensureNamespace(ctx, client, client.Namespace())
 	if err != nil {
-		return nil, err
+		return err
 	}
 	err = ensureAppCustomResource(ctx, client, a)
 	if err != nil {
-		return nil, err
+		return err
 	}
 
 	tclient, err := TsuruClientForConfig(client.restConfig)
 	if err != nil {
-		return nil, err
+		return err
 	}
 	appCRD, err := tclient.TsuruV1().Apps(client.Namespace()).Get(ctx, a.GetName(), metav1.GetOptions{})
 	if err != nil {
-		return nil, err
+		return err
 	}
+
+	originalAPPCRD := appCRD.DeepCopy()
 	appCRD.Spec.ServiceAccountName = serviceAccountNameForApp(a)
 
 	deploys, err := allDeploymentsForApp(ctx, client, a)
 	if err != nil {
-		return nil, err
+		return err
 	}
 	sort.Slice(deploys, func(i, j int) bool {
 		return deploys[i].Name < deploys[j].Name
 	})
 
 	svcs, err := allServicesForApp(ctx, client, a)
 	if err != nil {
-		return nil, err
+		return err
 	}
 	sort.Slice(svcs, func(i, j int) bool {
 		return svcs[i].Name < svcs[j].Name
@@ -1407,7 +1405,7 @@ func loadAndEnsureAppCustomResourceSynced(ctx context.Context, client *ClusterCl
 
 	pdbs, err := allPDBsForApp(ctx, client, a)
 	if err != nil {
-		return nil, err
+		return err
 	}
 	sort.Slice(pdbs, func(i, j int) bool {
 		return pdbs[i].Name < pdbs[j].Name
@@ -1420,17 +1418,22 @@ func loadAndEnsureAppCustomResourceSynced(ctx context.Context, client *ClusterCl
 
 	version, err := servicemanager.AppVersion.LatestSuccessfulVersion(ctx, a)
 	if err != nil && err != appTypes.ErrNoVersionsAvailable {
-		return nil, err
+		return err
 	}
 
 	if version != nil {
 		appCRD.Spec.Configs, err = normalizeConfigs(version)
 		if err != nil {
-			return nil, err
+			return err
 		}
 	}
 
-	return tclient.TsuruV1().Apps(client.Namespace()).Update(ctx, appCRD, metav1.UpdateOptions{})
+	if reflect.DeepEqual(originalAPPCRD.Spec, appCRD.Spec) {
+		return nil
+	}
+
+	_, err = tclient.TsuruV1().Apps(client.Namespace()).Update(ctx, appCRD, metav1.UpdateOptions{})
+	return err
 }
 
 func ensureAppCustomResource(ctx context.Context, client *ClusterClient, a provision.App) error {

provision/kubernetes/provisioner_test.go

@@ -2738,3 +2738,116 @@ func (s *S) TestProvisionerToggleRoutable(c *check.C) {
 	c.Assert(pods.Items, check.HasLen, 1)
 	c.Assert(pods.Items[0].Labels["tsuru.io/is-routable"], check.Equals, "true")
 }
+
+func (s *S) TestEnsureAppCustomResourceSyncedPreserveAnnotations(c *check.C) {
+	ctx := context.TODO()
+	a, wait, rollback := s.mock.DefaultReactions(c)
+	defer rollback()
+	version := newSuccessfulVersion(c, a, map[string]interface{}{
+		"processes": map[string]interface{}{
+			"web": "python myapp.py",
+		},
+	})
+	err := s.p.AddUnits(ctx, a, 1, "web", version, nil)
+	c.Assert(err, check.IsNil)
+	wait()
+
+	tclient, err := TsuruClientForConfig(s.clusterClient.restConfig)
+	c.Assert(err, check.IsNil)
+
+	foundTsuruApp, err := tclient.TsuruV1().Apps(s.clusterClient.Namespace()).Get(ctx, a.GetName(), metav1.GetOptions{})
+	c.Assert(err, check.IsNil)
+
+	foundTsuruApp.ObjectMeta.Annotations = map[string]string{
+		"external.io/teste": "true",
+	}
+
+	_, err = tclient.TsuruV1().Apps(s.clusterClient.Namespace()).Update(ctx, foundTsuruApp, metav1.UpdateOptions{})
+	c.Assert(err, check.IsNil)
+
+	err = ensureAppCustomResourceSynced(context.TODO(), s.clusterClient, a)
+	c.Assert(err, check.IsNil)
+
+	appCRD, err := tclient.TsuruV1().Apps(s.clusterClient.Namespace()).Get(ctx, a.GetName(), metav1.GetOptions{})
+	c.Assert(err, check.IsNil)
+
+	c.Assert(appCRD.ObjectMeta, check.DeepEquals, metav1.ObjectMeta{
+		Namespace: "tsuru",
+		Name:      "myapp",
+		Annotations: map[string]string{
+			"external.io/teste": "true",
+		},
+	})
+
+	c.Assert(appCRD.Spec, check.DeepEquals, tsuruv1.AppSpec{
+		NamespaceName:      "default",
+		ServiceAccountName: "app-myapp",
+		Deployments: map[string][]string{
+			"web": {"myapp-web"},
+		},
+		Services: map[string][]string{
+			"web": {"myapp-web", "myapp-web-units"},
+		},
+		PodDisruptionBudgets: map[string][]string{
+			"web": {"myapp-web"},
+		},
+	})
+
+}
+
+func (s *S) TestEnsureAppCustomResourceSyncedPreserveAnnotations2(c *check.C) {
+	ctx := context.TODO()
+	a, wait, rollback := s.mock.DefaultReactions(c)
+	defer rollback()
+	version := newSuccessfulVersion(c, a, map[string]interface{}{
+		"processes": map[string]interface{}{
+			"web": "python myapp.py",
+		},
+	})
+	err := s.p.AddUnits(ctx, a, 1, "web", version, nil)
+	c.Assert(err, check.IsNil)
+	wait()
+
+	tclient, err := TsuruClientForConfig(s.clusterClient.restConfig)
+	c.Assert(err, check.IsNil)
+
+	foundTsuruApp, err := tclient.TsuruV1().Apps(s.clusterClient.Namespace()).Get(ctx, a.GetName(), metav1.GetOptions{})
+	c.Assert(err, check.IsNil)
+
+	foundTsuruApp.ObjectMeta.Annotations = map[string]string{
+		"external.io/teste": "true",
+	}
+	foundTsuruApp.Spec.ServiceAccountName = "another" // this is the unique line different from previous test
+
+	_, err = tclient.TsuruV1().Apps(s.clusterClient.Namespace()).Update(ctx, foundTsuruApp, metav1.UpdateOptions{})
+	c.Assert(err, check.IsNil)
+
+	err = ensureAppCustomResourceSynced(context.TODO(), s.clusterClient, a)
+	c.Assert(err, check.IsNil)
+
+	appCRD, err := tclient.TsuruV1().Apps(s.clusterClient.Namespace()).Get(ctx, a.GetName(), metav1.GetOptions{})
+	c.Assert(err, check.IsNil)
+
+	c.Assert(appCRD.ObjectMeta, check.DeepEquals, metav1.ObjectMeta{
+		Namespace: "tsuru",
+		Name:      "myapp",
+		Annotations: map[string]string{
+			"external.io/teste": "true",
+		},
+	})
+
+	c.Assert(appCRD.Spec, check.DeepEquals, tsuruv1.AppSpec{
+		NamespaceName:      "default",
+		ServiceAccountName: "app-myapp",
+		Deployments: map[string][]string{
+			"web": {"myapp-web"},
+		},
+		Services: map[string][]string{
+			"web": {"myapp-web", "myapp-web-units"},
+		},
+		PodDisruptionBudgets: map[string][]string{
+			"web": {"myapp-web"},
+		},
+	})
+
+}