Skip to content

Commit

Permalink
Fix result of the APM Server controller (#1991)
Browse files Browse the repository at this point in the history
* Fix result of the APM server
  • Loading branch information
barkbay authored Oct 15, 2019
1 parent 65abf70 commit 318203f
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 10 deletions.
22 changes: 12 additions & 10 deletions pkg/controller/apmserver/apmserver_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,15 @@ func (r *ReconcileApmServer) doReconcile(request reconcile.Request, as *apmv1bet

state.UpdateApmServerExternalService(*svc)

return r.updateStatus(state)
// update status
err = r.updateStatus(state)
if err != nil && errors.IsConflict(err) {
log.V(1).Info("Conflict while updating status", "namespace", as.Namespace, "as", as.Name)
return reconcile.Result{Requeue: true}, nil
}
res, err := results.WithError(err).Aggregate()
k8s.EmitErrorEvent(r.recorder, err, as, events.EventReconciliationError, "Reconciliation error: %v", err)
return res, err
}

func (r *ReconcileApmServer) reconcileApmServerSecret(as *apmv1beta1.ApmServer) (*corev1.Secret, error) {
Expand Down Expand Up @@ -441,22 +449,16 @@ func (r *ReconcileApmServer) reconcileApmServerDeployment(
return state, nil
}

func (r *ReconcileApmServer) updateStatus(state State) (reconcile.Result, error) {
func (r *ReconcileApmServer) updateStatus(state State) error {
current := state.originalApmServer
if reflect.DeepEqual(current.Status, state.ApmServer.Status) {
return state.Result, nil
return nil
}
if state.ApmServer.Status.IsDegraded(current.Status) {
r.recorder.Event(current, corev1.EventTypeWarning, events.EventReasonUnhealthy, "Apm Server health degraded")
}
log.Info("Updating status", "namespace", state.ApmServer.Namespace, "as_name", state.ApmServer.Name, "iteration", atomic.LoadUint64(&r.iteration))
err := r.Status().Update(state.ApmServer)
if err != nil && errors.IsConflict(err) {
log.V(1).Info("Conflict while updating status")
return reconcile.Result{Requeue: true}, nil
}

return state.Result, err
return r.Status().Update(state.ApmServer)
}

// finalizersFor returns the list of finalizers applying to a given APM deployment
Expand Down
68 changes: 68 additions & 0 deletions pkg/controller/apmserver/apmserver_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"github.com/elastic/cloud-on-k8s/pkg/controller/common/defaults"
"github.com/elastic/cloud-on-k8s/pkg/controller/common/deployment"
"github.com/elastic/cloud-on-k8s/pkg/controller/common/keystore"
"github.com/elastic/cloud-on-k8s/pkg/controller/common/operator"
"github.com/elastic/cloud-on-k8s/pkg/controller/common/watches"
"github.com/elastic/cloud-on-k8s/pkg/utils/k8s"
"github.com/go-test/deep"
Expand All @@ -25,6 +26,7 @@ import (
"k8s.io/client-go/kubernetes/scheme"
"k8s.io/client-go/tools/record"
"sigs.k8s.io/controller-runtime/pkg/client/fake"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
)

var certSecretName = "test-apm-server-apm-http-certs-internal" // nolint
Expand Down Expand Up @@ -354,3 +356,69 @@ func TestReconcileApmServer_deploymentParams(t *testing.T) {
})
}
}

func TestReconcileApmServer_doReconcile(t *testing.T) {
require.NoError(t, apmv1beta1.AddToScheme(scheme.Scheme))
type fields struct {
resources []runtime.Object
recorder record.EventRecorder
dynamicWatches watches.DynamicWatches
Parameters operator.Parameters
}
type args struct {
request reconcile.Request
}
tests := []struct {
name string
as apmv1beta1.ApmServer
fields fields
args args
wantRequeue bool
wantErr bool
}{
{
name: "If no error ensure a requeue is scheduled for CA",
as: apmv1beta1.ApmServer{
ObjectMeta: metav1.ObjectMeta{
Name: "apmserver",
Namespace: "default",
},
},
fields: fields{
resources: []runtime.Object{},
recorder: record.NewFakeRecorder(100),
dynamicWatches: watches.NewDynamicWatches(),
Parameters: operator.Parameters{
CACertRotation: certificates.RotationParams{
Validity: certificates.DefaultCertValidity,
RotateBefore: certificates.DefaultRotateBefore,
},
},
},
args: args{
request: reconcile.Request{},
},
wantRequeue: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
r := &ReconcileApmServer{
Client: k8s.WrapClient(fake.NewFakeClientWithScheme(scheme.Scheme, &tt.as)),
scheme: scheme.Scheme,
recorder: tt.fields.recorder,
dynamicWatches: tt.fields.dynamicWatches,
Parameters: tt.fields.Parameters,
}
got, err := r.doReconcile(tt.args.request, tt.as.DeepCopy())
if (err != nil) != tt.wantErr {
t.Errorf("ReconcileApmServer.doReconcile() error = %v, wantErr %v", err, tt.wantErr)
return
}
require.NotNil(t, got)
require.Equal(t, got.Requeue, tt.wantRequeue)
// We just check that the requeue is not zero
require.True(t, got.RequeueAfter > 0)
})
}
}

0 comments on commit 318203f

Please sign in to comment.