Disable pruning for istios that potentially manage external revs (#654)

Signed-off-by: Nick Fox <[email protected]>

Author: nrfox

controllers/istio/istio_controller.go

@@ -91,7 +91,22 @@ func (r *Reconciler) doReconcile(ctx context.Context, istio *v1.Istio) (result c
 		return ctrl.Result{}, err
 	}
 
-	return revision.PruneInactive(ctx, r.Client, istio.UID, getActiveRevisionName(istio), getPruningGracePeriod(istio))
+	// We cannot prune revisions that manage an external cluster because the operator currently
+	// has no way of knowing if the revision is still in use on the external cluster.
+	if !managesExternalRevision(istio) {
+		return revision.PruneInactive(ctx, r.Client, istio.UID, getActiveRevisionName(istio), getPruningGracePeriod(istio))
+	}
+
+	return
+}
+
+func managesExternalRevision(istio *v1.Istio) bool {
+	if values := istio.Spec.Values; values != nil {
+		if pilot := values.Pilot; pilot != nil {
+			return pilot.Env["EXTERNAL_ISTIOD"] == "true"
+		}
+	}
+	return false
 }
 
 func validate(istio *v1.Istio) error {

controllers/istio/istio_controller_test.go

@@ -872,6 +872,47 @@ func TestGetPruningGracePeriod(t *testing.T) {
 	}
 }
 
+func TestManagesExternalRevision(t *testing.T) {
+	tests := []struct {
+		name     string
+		spec     v1.IstioSpec
+		expected bool
+	}{
+		{
+			name:     "Empty spec.values does not manage",
+			spec:     v1.IstioSpec{},
+			expected: false,
+		},
+		{
+			name:     "Empty spec.values.pilot does not manage",
+			spec:     v1.IstioSpec{Values: &v1.Values{}},
+			expected: false,
+		},
+		{
+			name:     "Empty spec.values.pilot.env does not manage",
+			spec:     v1.IstioSpec{Values: &v1.Values{Pilot: &v1.PilotConfig{}}},
+			expected: false,
+		},
+		{
+			name:     "env with EXTERNAL_ISTIOD=true does manage",
+			spec:     v1.IstioSpec{Values: &v1.Values{Pilot: &v1.PilotConfig{Env: map[string]string{"EXTERNAL_ISTIOD": "true"}}}},
+			expected: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			istio := &v1.Istio{
+				Spec: tt.spec,
+			}
+			got := managesExternalRevision(istio)
+			if got != tt.expected {
+				t.Errorf("managesExternalRevision() = %v, want %v", got, tt.expected)
+			}
+		})
+	}
+}
+
 func Must(t *testing.T, err error) {
 	t.Helper()
 	if err != nil {