Reconcile all clusters on license change (#2145)

License changes now propagate almost immediately to all clusters.
E2e test has been adjusted (and shortened) as a result of that.

pkg/controller/license/license_controller.go

@@ -115,16 +115,9 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error {
 			if !license.IsEnterpriseLicense(*secret) {
 				return nil
 			}
-			// TODO reconcile license secrets and augment license secret metadata with license UID to minimize parsing
-			license, err := license.ParseEnterpriseLicense(secret.Data)
-			if err != nil {
-				log.Error(err, "ignoring invalid or unparseable license in watch handler")
-				return nil
-			}
-
-			rs, err := listAffectedLicenses(
-				k8s.WrapClient(mgr.GetClient()), license.License.UID,
-			)
+			// if a license is added/modified we want to update for potentially all clusters managed by this instance
+			// of ECK which is why we are listing all Elasticsearch clusters here and trigger a reconciliation
+			rs, err := reconcileRequestsForAllClusters(k8s.WrapClient(mgr.GetClient()))
 			if err != nil {
 				// dropping the event(s) at this point
 				log.Error(err, "failed to list affected clusters in enterprise license watch")

pkg/controller/license/list.go

@@ -5,26 +5,23 @@
 package license
 
 import (
-	"github.com/elastic/cloud-on-k8s/pkg/controller/common/license"
+	"github.com/elastic/cloud-on-k8s/pkg/apis/elasticsearch/v1beta1"
 	"github.com/elastic/cloud-on-k8s/pkg/utils/k8s"
-	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 )
 
-func listAffectedLicenses(c k8s.Client, licenseName string) ([]reconcile.Request, error) {
-	var list = corev1.SecretList{}
-	// list all cluster licenses referencing the given enterprise license
-	matchLabels := license.NewLicenseByNameSelector(licenseName)
-	err := c.List(&list, matchLabels)
+func reconcileRequestsForAllClusters(c k8s.Client) ([]reconcile.Request, error) {
+	var clusters v1beta1.ElasticsearchList
+	// list all clusters
+	err := c.List(&clusters)
 	if err != nil {
 		return nil, err
 	}
 
-	// enqueue a reconcile request for each cluster license found leveraging the fact that cluster and license
-	// share the same name
-	requests := make([]reconcile.Request, len(list.Items))
-	for i, cl := range list.Items {
+	// create a reconcile request for each cluster
+	requests := make([]reconcile.Request, len(clusters.Items))
+	for i, cl := range clusters.Items {
 		requests[i] = reconcile.Request{NamespacedName: types.NamespacedName{
 			Namespace: cl.Namespace,
 			Name:      cl.Name,

pkg/controller/license/list_test.go

@@ -8,10 +8,9 @@ import (
 	"reflect"
 	"testing"
 
-	"github.com/elastic/cloud-on-k8s/pkg/controller/common/license"
+	"github.com/elastic/cloud-on-k8s/pkg/apis/elasticsearch/v1beta1"
 	"github.com/elastic/cloud-on-k8s/pkg/utils/k8s"
 	"github.com/pkg/errors"
-	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
@@ -34,7 +33,6 @@ func Test_listAffectedLicenses(t *testing.T) {
 	true := true
 
 	type args struct {
-		license        string
 		initialObjects []runtime.Object
 	}
 	tests := []struct {
@@ -47,33 +45,41 @@ func Test_listAffectedLicenses(t *testing.T) {
 		{
 			name: "happy path",
 			args: args{
-				license: "enterprise-license",
 				initialObjects: []runtime.Object{
-					&corev1.Secret{
+					&v1beta1.Elasticsearch{
 						ObjectMeta: metav1.ObjectMeta{
 							Name:      "foo-cluster",
 							Namespace: "default",
 							SelfLink:  "/apis/elasticsearch.k8s.elastic.co/",
-							Labels: map[string]string{
-								license.LicenseLabelName: "enterprise-license",
-							},
+						},
+					},
+					&v1beta1.Elasticsearch{
+						ObjectMeta: metav1.ObjectMeta{
+							Name:      "bar-cluster",
+							Namespace: "default",
+							SelfLink:  "/apis/elasticsearch.k8s.elastic.co/",
 						},
 					},
 				},
 			},
-			want: []reconcile.Request{{
-				NamespacedName: types.NamespacedName{
-					Namespace: "default",
-					Name:      "foo-cluster",
+			want: []reconcile.Request{
+				{
+					NamespacedName: types.NamespacedName{
+						Namespace: "default",
+						Name:      "foo-cluster",
+					},
 				},
-			}},
+				{
+					NamespacedName: types.NamespacedName{
+						Namespace: "default",
+						Name:      "bar-cluster",
+					},
+				}},
 			wantErr: false,
 		},
 		{
-			name: "list error",
-			args: args{
-				license: "bar",
-			},
+			name:          "list error",
+			args:          args{},
 			injectedError: errors.New("listing failed"),
 			wantErr:       true,
 		},
@@ -85,13 +91,13 @@ func Test_listAffectedLicenses(t *testing.T) {
 				client = &failingClient{Client: client, Error: tt.injectedError}
 			}
 
-			got, err := listAffectedLicenses(client, tt.args.license)
+			got, err := reconcileRequestsForAllClusters(client)
 			if (err != nil) != tt.wantErr {
-				t.Errorf("listAffectedLicenses() error = %v, wantErr %v", err, tt.wantErr)
+				t.Errorf("reconcileRequestsForAllClusters() error = %v, wantErr %v", err, tt.wantErr)
 				return
 			}
 			if !reflect.DeepEqual(got, tt.want) {
-				t.Errorf("listAffectedLicenses() = %v, want %v", got, tt.want)
+				t.Errorf("reconcileRequestsForAllClusters() = %v, want %v", got, tt.want)
 			}
 		})
 	}

test/e2e/es/license_test.go

@@ -28,10 +28,6 @@ func TestEnterpriseLicenseSingle(t *testing.T) {
 	esBuilder := elasticsearch.NewBuilder("test-es-license-provisioning").
 		WithESMasterDataNodes(1, elasticsearch.DefaultResources)
 
-	mutatedEsBuilder := esBuilder.
-		WithNoESTopology().
-		WithESMasterDataNodes(2, elasticsearch.DefaultResources)
-
 	licenseTestContext := elasticsearch.NewLicenseTestContext(k, esBuilder.Elasticsearch)
 
 	test.StepList{}.
@@ -44,14 +40,12 @@ func TestEnterpriseLicenseSingle(t *testing.T) {
 		WithSteps(test.StepList{
 			licenseTestContext.CheckElasticsearchLicense(license.ElasticsearchLicenseTypeBasic),
 			licenseTestContext.CreateEnterpriseLicenseSecret(licenseBytes),
+			// enterprise license can contain all kinds of cluster licenses so we are a bit lenient here and expect either gold or platinum
+			licenseTestContext.CheckElasticsearchLicense(
+				license.ElasticsearchLicenseTypeGold,
+				license.ElasticsearchLicenseTypePlatinum,
+			),
 		}).
-		// Mutation shortcuts the license provisioning check...
-		WithSteps(mutatedEsBuilder.MutationTestSteps(k)).
-		// enterprise license can contain all kinds of cluster licenses so we are a bit lenient here and expect either gold or platinum
-		WithStep(licenseTestContext.CheckElasticsearchLicense(
-			license.ElasticsearchLicenseTypeGold,
-			license.ElasticsearchLicenseTypePlatinum,
-		)).
-		WithSteps(mutatedEsBuilder.DeletionTestSteps(k)).
+		WithSteps(esBuilder.DeletionTestSteps(k)).
 		RunSequential(t)
 }