Skip to content

Commit

Permalink
Refactoring
Browse files Browse the repository at this point in the history
  • Loading branch information
@thbkrkr
thbkrkr committed Oct 4, 2021
1 parent 916e0dc commit 2fc7825
Show file tree
Hide file tree
Showing 5 changed files with 121 additions and 104 deletions.
18 changes: 11 additions & 7 deletions pkg/controller/elasticsearch/driver/driver.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@ import (
"context"
"crypto/x509"
"fmt"
"strings"
"time"

"github.com/pkg/errors"
corev1 "k8s.io/api/core/v1"
"k8s.io/client-go/tools/record"
controller "sigs.k8s.io/controller-runtime/pkg/reconcile"
Expand Down Expand Up @@ -195,16 +197,18 @@ func (d *defaultDriver) Reconcile(ctx context.Context) *reconciler.Results {
}

if esReachable { //nolint:nestif
// reconcile the license
requeueOnErr, err := license.Reconcile(ctx, d.Client, d.ES, esClient)
// reconcile the Elasticsearch license
unsupportedElasticsearch, err := license.Reconcile(ctx, d.Client, d.ES, esClient)
if err != nil {
msg := "Could not reconcile cluster license"
if !requeueOnErr {
if unsupportedElasticsearch {
msg := "Unsupported Elasticsearch"
d.ReconcileState.AddEvent(corev1.EventTypeWarning, events.EventReasonUnexpected, fmt.Sprintf("%s: %s", msg, err.Error()))
log.Error(err, msg, "namespace", d.ES.Namespace, "es_name", d.ES.Name)
// an error has been detected to get the license, let's update the phase to "invalid" and stop the reconciliation
d.ReconcileState.UpdateElasticsearchInvalid(err)
return results.WithError(err)
// unsupported Elasticsearch, let's update the phase to "invalid" and stop the reconciliation
d.ReconcileState.UpdateElasticsearchStatusPhase(esv1.ElasticsearchResourceInvalid)
return results.WithError(errors.Wrap(err, strings.ToLower(msg)))
}
msg := "Could not reconcile cluster license"
d.ReconcileState.AddEvent(corev1.EventTypeWarning, events.EventReasonUnexpected, fmt.Sprintf("%s: %s", msg, err.Error()))
log.Info(msg, "err", err, "namespace", d.ES.Namespace, "es_name", d.ES.Name)
results.WithResult(defaultRequeue)
Expand Down
32 changes: 12 additions & 20 deletions pkg/controller/elasticsearch/license/apply.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,39 +37,31 @@ func applyLinkedLicense(
c k8s.Client,
esCluster types.NamespacedName,
updater esclient.LicenseClient,
) (bool, error) {
requeueOnErr := true
// get the current license
current, err := updater.GetLicense(ctx)
if err != nil {
// do not requeue on 4xx, except 404 which may happen if the master node is generating a new cluster state
requeueOnErr = !esclient.Is4xx(err) || esclient.IsNotFound(err)
return requeueOnErr, fmt.Errorf("while getting current license level %w", err)
}

currentLicense esclient.License,
) error {
// get the expected license
// the underlying assumption here is that either a user or a
// license controller has created a cluster license in the
// namespace of this cluster following the cluster-license naming
// convention
var license corev1.Secret
err = c.Get(context.Background(),
err := c.Get(context.Background(),
types.NamespacedName{
Namespace: esCluster.Namespace,
Name: esv1.LicenseSecretName(esCluster.Name),
},
&license,
)
if err != nil && !apierrors.IsNotFound(err) {
return true, err
return err
}
if err != nil && apierrors.IsNotFound(err) {
// no license expected, let's look at the current cluster license
switch {
case isBasic(current):
case isBasic(currentLicense):
// nothing to do
return requeueOnErr, nil
case isTrial(current):
return nil
case isTrial(currentLicense):
// Elasticsearch reports a trial license, but there's no ECK enterprise trial requested.
// This can be the case if:
// - an ECK trial was started previously, then stopped (secret removed)
Expand All @@ -78,24 +70,24 @@ func applyLinkedLicense(
// we tolerate it to avoid a bad user experience because trials can only be started once.
log.V(1).Info("Preserving existing stack-level trial license",
"namespace", esCluster.Namespace, "es_name", esCluster.Name)
return requeueOnErr, nil
return nil
default:
// revert the current license to basic
return requeueOnErr, startBasic(ctx, updater)
return startBasic(ctx, updater)
}
}

bytes, err := commonlicense.FetchLicenseData(license.Data)
if err != nil {
return requeueOnErr, err
return err
}

var desired esclient.License
err = json.Unmarshal(bytes, &desired)
if err != nil {
return requeueOnErr, pkgerrors.Wrap(err, "no valid license found in license secret")
return pkgerrors.Wrap(err, "no valid license found in license secret")
}
return requeueOnErr, updateLicense(ctx, esCluster, updater, current, desired)
return updateLicense(ctx, esCluster, updater, currentLicense, desired)
}

func startBasic(ctx context.Context, updater esclient.LicenseClient) error {
Expand Down
151 changes: 75 additions & 76 deletions pkg/controller/elasticsearch/license/apply_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,17 +125,14 @@ func Test_applyLinkedLicense(t *testing.T) {
tests := []struct {
name string
initialObjs []runtime.Object
currentLicense esclient.License
errors map[client.ObjectKey]error
wantErr bool
wantRequeueOnErr bool
updater esclient.LicenseClient
clientAssertions func(updater fakeLicenseUpdater)
}{
{
name: "happy path",
wantErr: false,
wantRequeueOnErr: true,
updater: &fakeLicenseUpdater{},
name: "happy path",
wantErr: false,
initialObjs: []runtime.Object{
&corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -152,37 +149,32 @@ func Test_applyLinkedLicense(t *testing.T) {
},
},
{
name: "no error: no license found but stack has an enterprise license",
wantErr: false,
wantRequeueOnErr: true,
updater: &fakeLicenseUpdater{license: esclient.License{Type: string(esclient.ElasticsearchLicenseTypeEnterprise)}},
name: "no error: no license found but stack has an enterprise license",
wantErr: false,
currentLicense: esclient.License{Type: string(esclient.ElasticsearchLicenseTypeEnterprise)},
clientAssertions: func(updater fakeLicenseUpdater) {
require.True(t, updater.startBasicCalled, "should call start_basic to remove the license")
},
},
{
name: "no error: no license found, stack already in basic license",
wantErr: false,
wantRequeueOnErr: true,
updater: &fakeLicenseUpdater{license: esclient.License{Type: string(esclient.ElasticsearchLicenseTypeBasic)}},
name: "no error: no license found, stack already in basic license",
wantErr: false,
currentLicense: esclient.License{Type: string(esclient.ElasticsearchLicenseTypeBasic)},
clientAssertions: func(updater fakeLicenseUpdater) {
require.False(t, updater.startBasicCalled, "should not call start_basic if already basic")
},
},
{
name: "no error: no license found but tolerate a cluster level trial",
wantErr: false,
wantRequeueOnErr: true,
updater: &fakeLicenseUpdater{license: esclient.License{Type: string(esclient.ElasticsearchLicenseTypeTrial)}},
name: "no error: no license found but tolerate a cluster level trial",
wantErr: false,
currentLicense: esclient.License{Type: string(esclient.ElasticsearchLicenseTypeTrial)},
clientAssertions: func(updater fakeLicenseUpdater) {
require.False(t, updater.startBasicCalled, "should not call start_basic")
},
},
{
name: "error: empty license",
wantErr: true,
wantRequeueOnErr: true,
updater: &fakeLicenseUpdater{},
name: "error: empty license",
wantErr: true,
initialObjs: []runtime.Object{
&corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -193,10 +185,8 @@ func Test_applyLinkedLicense(t *testing.T) {
},
},
{
name: "error: invalid license json",
wantErr: true,
wantRequeueOnErr: true,
updater: &fakeLicenseUpdater{},
name: "error: invalid license json",
wantErr: true,
initialObjs: []runtime.Object{
&corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -210,46 +200,8 @@ func Test_applyLinkedLicense(t *testing.T) {
},
},
{
name: "error: request error",
wantErr: true,
wantRequeueOnErr: true,
updater: &fakeLicenseUpdater{},
errors: map[client.ObjectKey]error{
types.NamespacedName{
Namespace: clusterName.Namespace,
Name: esv1.LicenseSecretName("test"),
}: errors.New("boom"),
},
},
{
name: "do not requeue on error 400 on get license",
wantErr: true,
wantRequeueOnErr: false,
updater: &fakeInvalidLicenseUpdater{statusCodeOnGetLicense: 400},
errors: map[client.ObjectKey]error{
types.NamespacedName{
Namespace: clusterName.Namespace,
Name: esv1.LicenseSecretName("test"),
}: errors.New("boom"),
},
},
{
name: "requeue on error 404 on get license",
wantErr: true,
wantRequeueOnErr: true,
updater: &fakeInvalidLicenseUpdater{statusCodeOnGetLicense: 404},
errors: map[client.ObjectKey]error{
types.NamespacedName{
Namespace: clusterName.Namespace,
Name: esv1.LicenseSecretName("test"),
}: errors.New("boom"),
},
},
{
name: "requeue on error 500 on get license",
wantErr: true,
wantRequeueOnErr: true,
updater: &fakeInvalidLicenseUpdater{statusCodeOnGetLicense: 500},
name: "error: request error",
wantErr: true,
errors: map[client.ObjectKey]error{
types.NamespacedName{
Namespace: clusterName.Namespace,
Expand All @@ -264,22 +216,66 @@ func Test_applyLinkedLicense(t *testing.T) {
Client: k8s.NewFakeClient(tt.initialObjs...),
errors: tt.errors,
}
requeueOnErr, err := applyLinkedLicense(
updater := fakeLicenseUpdater{license: tt.currentLicense}
if err := applyLinkedLicense(
context.Background(),
c,
clusterName,
tt.updater,
)
if (err != nil) != tt.wantErr {
&updater,
tt.currentLicense,
); (err != nil) != tt.wantErr {
t.Errorf("applyLinkedLicense() error = %v, wantErr %v", err, tt.wantErr)
}
if requeueOnErr != tt.wantRequeueOnErr {
t.Errorf("applyLinkedLicense() requeueOnErr = %v, wantRequeueOnErr %v", requeueOnErr, tt.wantRequeueOnErr)
}
if tt.clientAssertions != nil {
if flu, ok := tt.updater.(*fakeLicenseUpdater); ok {
tt.clientAssertions(*flu)
}
tt.clientAssertions(updater)
}
})
}
}

func Test_checkEsLicense(t *testing.T) {
tests := []struct {
name string
wantErr bool
unsupported bool
updater esclient.LicenseClient
}{
{
name: "happy path",
wantErr: false,
unsupported: false,
updater: &fakeInvalidLicenseUpdater{
fakeLicenseUpdater: &fakeLicenseUpdater{license: esclient.License{Type: string(esclient.ElasticsearchLicenseTypeBasic)}},
statusCodeOnGetLicense: 200,
},
},
{
name: "error: 400 on get license, unsupported",
wantErr: true,
unsupported: true,
updater: &fakeInvalidLicenseUpdater{statusCodeOnGetLicense: 400},
},
{
name: "error: 404 on get license, supported",
wantErr: true,
unsupported: false,
updater: &fakeInvalidLicenseUpdater{statusCodeOnGetLicense: 404},
},
{
name: "error: 500 on get license, supported",
wantErr: true,
unsupported: false,
updater: &fakeInvalidLicenseUpdater{statusCodeOnGetLicense: 500},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
_, unsupported, err := checkEsLicense(context.Background(), tt.updater)
if (err != nil) != tt.wantErr {
t.Errorf("checkEsLicense() error = %v, wantErr %v", err, tt.wantErr)
}
if unsupported != tt.unsupported {
t.Errorf("checkEsLicense() unsupported = %v, unsupported %v", unsupported, tt.unsupported)
}
})
}
Expand All @@ -291,6 +287,9 @@ type fakeInvalidLicenseUpdater struct {
}

func (f *fakeInvalidLicenseUpdater) GetLicense(ctx context.Context) (esclient.License, error) {
if f.statusCodeOnGetLicense == 200 {
return f.license, nil
}
apiErr := esclient.FakeAPIError(f.statusCodeOnGetLicense)
return esclient.License{}, &apiErr
}
Expand Down
20 changes: 19 additions & 1 deletion pkg/controller/elasticsearch/license/reconcile.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,24 @@ func Reconcile(
esCluster esv1.Elasticsearch,
clusterClient esclient.Client,
) (bool, error) {
currentLicense, unsupportedElasticsearch, err := checkElasticsearchLicense(ctx, clusterClient)
if err != nil {
return unsupportedElasticsearch, err
}

clusterName := k8s.ExtractNamespacedName(&esCluster)
return applyLinkedLicense(ctx, c, clusterName, clusterClient)
return true, applyLinkedLicense(ctx, c, clusterName, clusterClient, currentLicense)
}

// checkElasticsearchLicense checks that Elasticsearch is licensed, which ensures that the operator is communicating
// with a supported version of Elasticsearch
func checkElasticsearchLicense(ctx context.Context, clusterClient esclient.LicenseClient) (esclient.License, bool, error) {
currentLicense, err := clusterClient.GetLicense(ctx)
if err != nil {
// 4xx is not supported, except 404 which may happen if the master node is generating a new cluster state
unsupportedElasticsearch := esclient.Is4xx(err) && !esclient.IsNotFound(err)
return esclient.License{}, unsupportedElasticsearch, err
}

return currentLicense, false, nil
}
4 changes: 4 additions & 0 deletions pkg/controller/elasticsearch/reconcile/state.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,3 +153,7 @@ func (s *State) UpdateElasticsearchInvalid(err error) {
s.status.Phase = esv1.ElasticsearchResourceInvalid
s.AddEvent(corev1.EventTypeWarning, events.EventReasonValidation, err.Error())
}

func (s *State) UpdateElasticsearchStatusPhase(orchPhase esv1.ElasticsearchOrchestrationPhase) {
s.status.Phase = orchPhase
}

0 comments on commit 2fc7825

Please sign in to comment.