Cherry pick Brokercell annotation updates to release 0.17 (#1904)

* Cherry pick brokercell istio

* Cherry pick #1656

* Cherry pick #1775

Co-authored-by: cshou <cshou@google.com>

pkg/reconciler/brokercell/brokercell.go

@@ -239,6 +239,7 @@ func (r *Reconciler) makeIngressArgs(bc *intv1alpha1.BrokerCell) resources.Ingre
 			CPULimit:           *bc.Spec.Components.Ingress.Resources.Limits.CPU,
 			MemoryRequest:      *bc.Spec.Components.Ingress.Resources.Requests.Memory,
 			MemoryLimit:        *bc.Spec.Components.Ingress.Resources.Limits.Memory,
+			RolloutRestartTime: bc.GetAnnotations()[resources.IngressRestartTimeAnnotationKey],
 		},
 		Port: r.env.IngressPort,
 	}
@@ -267,6 +268,8 @@ func (r *Reconciler) makeFanoutArgs(bc *intv1alpha1.BrokerCell) resources.Fanout
 			CPULimit:           *bc.Spec.Components.Fanout.Resources.Limits.CPU,
 			MemoryRequest:      *bc.Spec.Components.Fanout.Resources.Requests.Memory,
 			MemoryLimit:        *bc.Spec.Components.Fanout.Resources.Limits.Memory,
+			AllowIstioSidecar:  true,
+			RolloutRestartTime: bc.GetAnnotations()[resources.FanoutRestartTimeAnnotationKey],
 		},
 	}
 }
@@ -294,6 +297,8 @@ func (r *Reconciler) makeRetryArgs(bc *intv1alpha1.BrokerCell) resources.RetryAr
 			CPULimit:           *bc.Spec.Components.Retry.Resources.Limits.CPU,
 			MemoryRequest:      *bc.Spec.Components.Retry.Resources.Requests.Memory,
 			MemoryLimit:        *bc.Spec.Components.Retry.Resources.Limits.Memory,
+			AllowIstioSidecar:  true,
+			RolloutRestartTime: bc.GetAnnotations()[resources.RetryRestartTimeAnnotationKey],
 		},
 	}
 }

pkg/reconciler/brokercell/brokercell_test.go

@@ -57,7 +57,12 @@ const (
 var (
 	testKey = fmt.Sprintf("%s/%s", testNS, brokerCellName)
 
-	creatorAnnotation = map[string]string{"internal.events.cloud.google.com/creator": "googlecloud"}
+	creatorAnnotation       = map[string]string{"internal.events.cloud.google.com/creator": "googlecloud"}
+	restartedTimeAnnotation = map[string]string{
+		"events.cloud.google.com/ingressRestartRequestedAt": "2020-09-25T16:28:36-04:00",
+		"events.cloud.google.com/fanoutRestartRequestedAt":  "2020-09-25T16:28:36-04:00",
+		"events.cloud.google.com/retryRestartRequestedAt":   "2020-09-25T16:28:36-04:00",
+	}
 
 	brokerCellReconciledEvent     = Eventf(corev1.EventTypeNormal, "BrokerCellReconciled", `BrokerCell reconciled: "testnamespace/test-brokercell"`)
 	brokerCellGCEvent             = Eventf(corev1.EventTypeNormal, "BrokerCellGarbageCollected", `BrokerCell garbage collected: "testnamespace/test-brokercell"`)
@@ -879,6 +884,42 @@ func TestAllCases(t *testing.T) {
 				},
 			},
 			WantEvents: []string{brokerCellGCEvent},
+		}, {
+			Name: "Brokercell has restart time annotation, deployments are updated with restart time annotation successfully",
+			Key:  testKey,
+			Objects: []runtime.Object{
+				NewBrokerCell(brokerCellName, testNS, WithBrokerCellSetDefaults,
+					WithBrokerCellAnnotations(restartedTimeAnnotation)),
+				testingdata.EmptyConfig(t, NewBrokerCell(brokerCellName, testNS, WithBrokerCellSetDefaults)),
+				NewEndpoints(brokerCellName+"-brokercell-ingress", testNS,
+					WithEndpointsAddresses(corev1.EndpointAddress{IP: "127.0.0.1"})),
+				testingdata.IngressDeploymentWithStatus(t),
+				testingdata.IngressServiceWithStatus(t),
+				testingdata.FanoutDeploymentWithStatus(t),
+				testingdata.RetryDeploymentWithStatus(t),
+				testingdata.IngressHPA(t),
+				testingdata.FanoutHPA(t),
+				testingdata.RetryHPA(t),
+			},
+			WantStatusUpdates: []clientgotesting.UpdateActionImpl{{
+				Object: NewBrokerCell(brokerCellName, testNS,
+					WithBrokerCellReady,
+					WithBrokerCellAnnotations(restartedTimeAnnotation),
+					WithIngressTemplate("http://test-brokercell-brokercell-ingress.testnamespace.svc.cluster.local/{namespace}/{name}"),
+					WithBrokerCellSetDefaults,
+				),
+			}},
+			WantUpdates: []clientgotesting.UpdateActionImpl{
+				{Object: testingdata.IngressDeploymentWithRestartAnnotation(t)},
+				{Object: testingdata.FanoutDeploymentWithRestartAnnotation(t)},
+				{Object: testingdata.RetryDeploymentWithRestartAnnotation(t)},
+			},
+			WantEvents: []string{
+				ingressDeploymentUpdatedEvent,
+				fanoutDeploymentUpdatedEvent,
+				retryDeploymentUpdatedEvent,
+				brokerCellReconciledEvent,
+			},
 		},
 	}
 

pkg/reconciler/brokercell/resources/args.go

@@ -32,6 +32,12 @@ const (
 	// RetryName is the name used for the retry container.
 	RetryName          = "retry"
 	BrokerCellLabelKey = "brokerCell"
+
+	// the annotation key of the time to trigger rollout restart of the deployments
+	IngressRestartTimeAnnotationKey = "events.cloud.google.com/ingressRestartRequestedAt"
+	FanoutRestartTimeAnnotationKey  = "events.cloud.google.com/fanoutRestartRequestedAt"
+	RetryRestartTimeAnnotationKey   = "events.cloud.google.com/retryRestartRequestedAt"
+	RolloutRestartTimeAnnotationKey = "events.cloud.google.com/RestartRequestedAt"
 )
 
 var (
@@ -50,6 +56,7 @@ type Args struct {
 	CPULimit           string
 	MemoryRequest      string
 	MemoryLimit        string
+	RolloutRestartTime string
 }
 
 // IngressArgs are the arguments to create a Broker's ingress Deployment.

pkg/reconciler/brokercell/resources/deployments.go

@@ -26,6 +26,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/intstr"
 	"knative.dev/pkg/kmeta"
+	"knative.dev/pkg/ptr"
 	"knative.dev/pkg/system"
 )
 
@@ -44,7 +45,7 @@ func MakeIngressDeployment(args IngressArgs) *appsv1.Deployment {
 			},
 		},
 		FailureThreshold: 3,
-		PeriodSeconds:    2,
+		PeriodSeconds:    15,
 		SuccessThreshold: 1,
 		TimeoutSeconds:   5,
 	}
@@ -57,8 +58,8 @@ func MakeIngressDeployment(args IngressArgs) *appsv1.Deployment {
 			},
 		},
 		FailureThreshold:    3,
-		InitialDelaySeconds: 5,
-		PeriodSeconds:       2,
+		InitialDelaySeconds: 15,
+		PeriodSeconds:       15,
 		SuccessThreshold:    1,
 		TimeoutSeconds:      5,
 	}
@@ -126,6 +127,12 @@ func MakeRetryDeployment(args RetryArgs) *appsv1.Deployment {
 
 // deploymentTemplate creates a template for data plane deployments.
 func deploymentTemplate(args Args, containers []corev1.Container) *appsv1.Deployment {
+	annotation := map[string]string{
+		"sidecar.istio.io/inject": strconv.FormatBool(args.AllowIstioSidecar),
+	}
+	if args.RolloutRestartTime != "" {
+		annotation[RolloutRestartTimeAnnotationKey] = args.RolloutRestartTime
+	}
 	return &appsv1.Deployment{
 		ObjectMeta: metav1.ObjectMeta{
 			Namespace:       args.BrokerCell.Namespace,
@@ -135,12 +142,17 @@ func deploymentTemplate(args Args, containers []corev1.Container) *appsv1.Deploy
 		},
 		Spec: appsv1.DeploymentSpec{
 			Selector: &metav1.LabelSelector{MatchLabels: Labels(args.BrokerCell.Name, args.ComponentName)},
+			Strategy: appsv1.DeploymentStrategy{
+				RollingUpdate: &appsv1.RollingUpdateDeployment{
+					MaxSurge:       &intstr.IntOrString{IntVal: 1},
+					MaxUnavailable: &intstr.IntOrString{IntVal: 0},
+				},
+			},
+			MinReadySeconds: 60,
 			Template: corev1.PodTemplateSpec{
 				ObjectMeta: metav1.ObjectMeta{
-					Labels: Labels(args.BrokerCell.Name, args.ComponentName),
-					Annotations: map[string]string{
-						"sidecar.istio.io/inject": strconv.FormatBool(args.AllowIstioSidecar),
-					},
+					Labels:      Labels(args.BrokerCell.Name, args.ComponentName),
+					Annotations: annotation,
 				},
 				Spec: corev1.PodSpec{
 					ServiceAccountName: args.ServiceAccountName,
@@ -154,7 +166,8 @@ func deploymentTemplate(args Args, containers []corev1.Container) *appsv1.Deploy
 							VolumeSource: corev1.VolumeSource{Secret: &corev1.SecretVolumeSource{SecretName: "google-broker-key", Optional: &optionalSecretVolume}},
 						},
 					},
-					Containers: containers,
+					Containers:                    containers,
+					TerminationGracePeriodSeconds: ptr.Int64(60),
 				},
 			},
 		},

pkg/reconciler/brokercell/testingdata/fanout_deployment.yaml

@@ -32,13 +32,19 @@ spec:
       app: cloud-run-events
       brokerCell: test-brokercell
       role: fanout
+  minReadySeconds: 60
+  strategy:
+    rollingUpdate:
+      maxUnavailable: 0
+      maxSurge: 1
   template:
     metadata:
       labels: *labels
       annotations:
-        sidecar.istio.io/inject: "false"
+        sidecar.istio.io/inject: "true"
     spec:
       serviceAccountName: broker
+      terminationGracePeriodSeconds: 60
       containers:
       - name: fanout
         image: fanout

pkg/reconciler/brokercell/testingdata/fanout_deployment_with_restart_annotation.yaml

@@ -0,0 +1,107 @@
+# Copyright 2020 Google LLC
+
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+
+#     http://www.apache.org/licenses/LICENSE-2.0
+
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# This yaml matches the fanout deployment objected created by the reconciler with
+# additional status so that reconciler will mark readiness based on the status.
+metadata:
+  name: test-brokercell-brokercell-fanout
+  namespace: testnamespace
+  labels:
+    app: cloud-run-events
+    brokerCell: test-brokercell
+    role: fanout
+  ownerReferences:
+    - apiVersion: internal.events.cloud.google.com/v1alpha1
+      kind: BrokerCell
+      name: test-brokercell
+      controller: true
+      blockOwnerDeletion: true
+spec:
+  selector:
+    matchLabels: &labels
+      app: cloud-run-events
+      brokerCell: test-brokercell
+      role: fanout
+  minReadySeconds: 60
+  strategy:
+    rollingUpdate:
+      maxUnavailable: 0
+      maxSurge: 1
+  template:
+    metadata:
+      labels: *labels
+      annotations:
+        sidecar.istio.io/inject: "true"
+        events.cloud.google.com/RestartRequestedAt: "2020-09-25T16:28:36-04:00"
+    spec:
+      serviceAccountName: broker
+      terminationGracePeriodSeconds: 60
+      containers:
+        - name: fanout
+          image: fanout
+          livenessProbe:
+            failureThreshold: 3
+            httpGet:
+              path: /healthz
+              port: 8080
+              scheme: HTTP
+            initialDelaySeconds: 15
+            periodSeconds: 15
+            successThreshold: 1
+            timeoutSeconds: 5
+          env:
+            - name: GOOGLE_APPLICATION_CREDENTIALS
+              value: /var/secrets/google/key.json
+            - name: SYSTEM_NAMESPACE
+              value: knative-testing
+            - name: POD_NAME
+              valueFrom:
+                fieldRef:
+                  fieldPath: metadata.name
+            - name: CONFIG_LOGGING_NAME
+              value: config-logging
+            - name: CONFIG_OBSERVABILITY_NAME
+              value: config-observability
+            - name: METRICS_DOMAIN
+              value: knative.dev/internal/eventing
+            - name: MAX_CONCURRENCY_PER_EVENT
+              value: "100"
+          volumeMounts:
+            - name: broker-config
+              mountPath: /var/run/cloud-run-events/broker
+            - name: google-broker-key
+              mountPath: /var/secrets/google
+          resources:
+            limits:
+              memory: 3000Mi
+            requests:
+              cpu: 1500m
+              memory: 500Mi
+          ports:
+            - name: metrics
+              containerPort: 9090
+            - name: http-health
+              containerPort: 8080
+      volumes:
+        - name: broker-config
+          configMap:
+            name: test-brokercell-brokercell-broker-targets
+        - name: google-broker-key
+          secret:
+            secretName: google-broker-key
+            optional: true
+status:
+  conditions:
+    - status: "True"
+      type: Available

pkg/reconciler/brokercell/testingdata/fanout_deployment_with_status.yaml

@@ -33,13 +33,19 @@ spec:
       app: cloud-run-events
       brokerCell: test-brokercell
       role: fanout
+  minReadySeconds: 60
+  strategy:
+    rollingUpdate:
+      maxUnavailable: 0
+      maxSurge: 1
   template:
     metadata:
       labels: *labels
       annotations:
-        sidecar.istio.io/inject: "false"
+        sidecar.istio.io/inject: "true"
     spec:
       serviceAccountName: broker
+      terminationGracePeriodSeconds: 60
       containers:
       - name: fanout
         image: fanout

pkg/reconciler/brokercell/testingdata/ingress_deployment.yaml

@@ -32,13 +32,19 @@ spec:
       app: cloud-run-events
       brokerCell: test-brokercell
       role: ingress
+  minReadySeconds: 60
+  strategy:
+    rollingUpdate:
+      maxUnavailable: 0
+      maxSurge: 1
   template:
     metadata:
       labels: *labels
       annotations:
         sidecar.istio.io/inject: "true"
     spec:
       serviceAccountName: broker
+      terminationGracePeriodSeconds: 60
       containers:
       - name: ingress
         image: ingress
@@ -48,8 +54,8 @@ spec:
             path: /healthz
             port: 8080
             scheme: HTTP
-          initialDelaySeconds: 5
-          periodSeconds: 2
+          initialDelaySeconds: 15
+          periodSeconds: 15
           successThreshold: 1
           timeoutSeconds: 5
         readinessProbe:
@@ -58,7 +64,7 @@ spec:
             path: /healthz
             port: 8080
             scheme: HTTP
-          periodSeconds: 2
+          periodSeconds: 15
           successThreshold: 1
           timeoutSeconds: 5
         env: