From 9bf00354a1d25ad8fe07058291e5ea659042690e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juraci=20Paix=C3=A3o=20Kr=C3=B6hling?= Date: Tue, 3 Dec 2019 13:43:16 +0100 Subject: [PATCH 1/2] Added basic status to CR{D} MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Juraci Paixão Kröhling --- deploy/crds/jaegertracing.io_jaegers_crd.yaml | 11 ++++++++ pkg/apis/jaegertracing/v1/jaeger_types.go | 18 ++++++++++++ pkg/controller/jaeger/jaeger_controller.go | 28 +++++++++++++++++++ .../jaeger/jaeger_controller_test.go | 10 +++++-- 4 files changed, 64 insertions(+), 3 deletions(-) diff --git a/deploy/crds/jaegertracing.io_jaegers_crd.yaml b/deploy/crds/jaegertracing.io_jaegers_crd.yaml index b7804e4e7..b50c9a851 100644 --- a/deploy/crds/jaegertracing.io_jaegers_crd.yaml +++ b/deploy/crds/jaegertracing.io_jaegers_crd.yaml @@ -3,6 +3,15 @@ kind: CustomResourceDefinition metadata: name: jaegers.jaegertracing.io spec: + additionalPrinterColumns: + - JSONPath: .status.phase + description: Instance's status + name: Status + type: string + - JSONPath: .status.version + description: Jaeger Version + name: Version + type: string group: jaegertracing.io names: kind: Jaeger @@ -10,6 +19,8 @@ spec: plural: jaegers singular: jaeger scope: Namespaced + subresources: + status: {} version: v1 versions: - name: v1 diff --git a/pkg/apis/jaegertracing/v1/jaeger_types.go b/pkg/apis/jaegertracing/v1/jaeger_types.go index 4cf1a9ea8..7a7b54487 100644 --- a/pkg/apis/jaegertracing/v1/jaeger_types.go +++ b/pkg/apis/jaegertracing/v1/jaeger_types.go @@ -11,6 +11,10 @@ import ( // +k8s:openapi-gen=true type IngressSecurityType string +// JaegerPhase represents the current phase of Jaeger instances +// +k8s:openapi-gen=true +type JaegerPhase string + const ( // FlagPlatformKubernetes represents the value for the 'platform' flag for Kubernetes // +k8s:openapi-gen=true @@ -67,6 +71,14 @@ const ( // AnnotationProvisionedKafkaValue is a label to be added to Kafkas that have been provisioned by Jaeger // +k8s:openapi-gen=true AnnotationProvisionedKafkaValue string = "true" + + // JaegerPhaseFailed indicates that the Jaeger instance failed to be provisioned + // +k8s:openapi-gen=true + JaegerPhaseFailed JaegerPhase = "Failed" + + // JaegerPhaseRunning indicates that the Jaeger instance is ready and running + // +k8s:openapi-gen=true + JaegerPhaseRunning JaegerPhase = "Running" ) // JaegerSpec defines the desired state of Jaeger @@ -110,12 +122,18 @@ type JaegerSpec struct { // +k8s:openapi-gen=true type JaegerStatus struct { Version string `json:"version"` + + // +kubebuilder:validation:Default=Pending + Phase JaegerPhase `json:"phase"` } // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object // Jaeger is the Schema for the jaegers API // +k8s:openapi-gen=true +// +kubebuilder:subresource:status +// +kubebuilder:printcolumn:name="Status",type="string",JSONPath=".status.phase",description="Instance's status" +// +kubebuilder:printcolumn:name="Version",type="string",JSONPath=".status.version",description="Jaeger Version" type Jaeger struct { metav1.TypeMeta `json:",inline"` diff --git a/pkg/controller/jaeger/jaeger_controller.go b/pkg/controller/jaeger/jaeger_controller.go index 6191c972d..a5f042608 100644 --- a/pkg/controller/jaeger/jaeger_controller.go +++ b/pkg/controller/jaeger/jaeger_controller.go @@ -138,6 +138,13 @@ func (r *ReconcileJaeger) Reconcile(request reconcile.Request) (reconcile.Result instance.Labels[v1.LabelOperatedBy] = identity if err := r.client.Update(ctx, instance); err != nil { + // update the status to "Failed" + instance.Status.Phase = v1.JaegerPhaseFailed + if err := r.client.Status().Update(ctx, instance); err != nil { + // we let it return the real error later + logFields.WithError(err).Error("failed to store the failed status into the current CustomResource after setting the identity") + } + logFields.WithField( "operator-identity", identity, ).WithError(err).Error("failed to set this operator as the manager of the instance") @@ -160,12 +167,24 @@ func (r *ReconcileJaeger) Reconcile(request reconcile.Request) (reconcile.Result }) list := &corev1.SecretList{} if err := r.client.List(ctx, list, opts); err != nil { + instance.Status.Phase = v1.JaegerPhaseFailed + if err := r.client.Status().Update(ctx, instance); err != nil { + // we let it return the real error later + logFields.WithError(err).Error("failed to store the failed status into the current CustomResource after preconditions") + } return reconcile.Result{}, tracing.HandleError(err, span) } str := r.runStrategyChooser(ctx, instance, list.Items) updated, err := r.apply(ctx, *instance, str) if err != nil { + // update the status to "Failed" + instance.Status.Phase = v1.JaegerPhaseFailed + if err := r.client.Status().Update(ctx, instance); err != nil { + // we let it return the real error later + logFields.WithError(err).Error("failed to store the failed status into the current CustomResource after the reconciliation") + } + logFields.WithError(err).Error("failed to apply the changes") return reconcile.Result{}, tracing.HandleError(err, span) } @@ -179,6 +198,15 @@ func (r *ReconcileJaeger) Reconcile(request reconcile.Request) (reconcile.Result } } + // update the status to "Ready" + if instance.Status.Phase != v1.JaegerPhaseRunning { + instance.Status.Phase = v1.JaegerPhaseRunning + if err := r.client.Status().Update(ctx, instance); err != nil { + logFields.WithError(err).Error("failed to store the status into the current CustomResource") + return reconcile.Result{}, tracing.HandleError(err, span) + } + } + log.WithFields(log.Fields{ "namespace": request.Namespace, "instance": request.Name, diff --git a/pkg/controller/jaeger/jaeger_controller_test.go b/pkg/controller/jaeger/jaeger_controller_test.go index 50f200c4a..2546fa768 100644 --- a/pkg/controller/jaeger/jaeger_controller_test.go +++ b/pkg/controller/jaeger/jaeger_controller_test.go @@ -53,9 +53,10 @@ func TestNewJaegerInstance(t *testing.T) { assert.NoError(t, err) // these are filled with default values - // TODO(jpkroehling): enable the assertion when the following issue is fixed: - // https://github.com/jaegertracing/jaeger-operator/issues/231 - // assert.Equal(t, "custom-strategy", persisted.Spec.Strategy) + assert.Equal(t, v1.DeploymentStrategyAllInOne, persisted.Spec.Strategy) + + // the status object got updated as well + assert.Equal(t, v1.JaegerPhaseRunning, persisted.Status.Phase) } func TestDeletedInstance(t *testing.T) { @@ -145,6 +146,9 @@ func TestSkipOnNonOwnedCR(t *testing.T) { // the only way to reliably test this is to verify that the operator didn't attempt to set the ownership field assert.Equal(t, "another-identity", persisted.Labels[v1.LabelOperatedBy]) + + // condition shouldn't be touched when the object belongs to another operator + assert.Equal(t, v1.JaegerPhase(""), persisted.Status.Phase) } func getReconciler(objs []runtime.Object) (*ReconcileJaeger, client.Client) { From 8c5197c798303c600af27bc01e8d0540cc3a9822 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juraci=20Paix=C3=A3o=20Kr=C3=B6hling?= Date: Wed, 4 Dec 2019 14:00:45 +0100 Subject: [PATCH 2/2] Changes based on the code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Juraci Paixão Kröhling --- deploy/crds/jaegertracing.io_jaegers_crd.yaml | 2 +- pkg/apis/jaegertracing/v1/jaeger_types.go | 8 +++----- pkg/controller/jaeger/jaeger_controller.go | 2 +- pkg/controller/jaeger/jaeger_controller_test.go | 2 -- 4 files changed, 5 insertions(+), 9 deletions(-) diff --git a/deploy/crds/jaegertracing.io_jaegers_crd.yaml b/deploy/crds/jaegertracing.io_jaegers_crd.yaml index b50c9a851..dc4890791 100644 --- a/deploy/crds/jaegertracing.io_jaegers_crd.yaml +++ b/deploy/crds/jaegertracing.io_jaegers_crd.yaml @@ -5,7 +5,7 @@ metadata: spec: additionalPrinterColumns: - JSONPath: .status.phase - description: Instance's status + description: Jaeger instance's status name: Status type: string - JSONPath: .status.version diff --git a/pkg/apis/jaegertracing/v1/jaeger_types.go b/pkg/apis/jaegertracing/v1/jaeger_types.go index 7a7b54487..5e9fc461e 100644 --- a/pkg/apis/jaegertracing/v1/jaeger_types.go +++ b/pkg/apis/jaegertracing/v1/jaeger_types.go @@ -121,10 +121,8 @@ type JaegerSpec struct { // JaegerStatus defines the observed state of Jaeger // +k8s:openapi-gen=true type JaegerStatus struct { - Version string `json:"version"` - - // +kubebuilder:validation:Default=Pending - Phase JaegerPhase `json:"phase"` + Version string `json:"version"` + Phase JaegerPhase `json:"phase"` } // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object @@ -132,7 +130,7 @@ type JaegerStatus struct { // Jaeger is the Schema for the jaegers API // +k8s:openapi-gen=true // +kubebuilder:subresource:status -// +kubebuilder:printcolumn:name="Status",type="string",JSONPath=".status.phase",description="Instance's status" +// +kubebuilder:printcolumn:name="Status",type="string",JSONPath=".status.phase",description="Jaeger instance's status" // +kubebuilder:printcolumn:name="Version",type="string",JSONPath=".status.version",description="Jaeger Version" type Jaeger struct { metav1.TypeMeta `json:",inline"` diff --git a/pkg/controller/jaeger/jaeger_controller.go b/pkg/controller/jaeger/jaeger_controller.go index a5f042608..2a5dafd9b 100644 --- a/pkg/controller/jaeger/jaeger_controller.go +++ b/pkg/controller/jaeger/jaeger_controller.go @@ -202,7 +202,7 @@ func (r *ReconcileJaeger) Reconcile(request reconcile.Request) (reconcile.Result if instance.Status.Phase != v1.JaegerPhaseRunning { instance.Status.Phase = v1.JaegerPhaseRunning if err := r.client.Status().Update(ctx, instance); err != nil { - logFields.WithError(err).Error("failed to store the status into the current CustomResource") + logFields.WithError(err).Error("failed to store the running status into the current CustomResource") return reconcile.Result{}, tracing.HandleError(err, span) } } diff --git a/pkg/controller/jaeger/jaeger_controller_test.go b/pkg/controller/jaeger/jaeger_controller_test.go index 2546fa768..e47ee0e99 100644 --- a/pkg/controller/jaeger/jaeger_controller_test.go +++ b/pkg/controller/jaeger/jaeger_controller_test.go @@ -146,8 +146,6 @@ func TestSkipOnNonOwnedCR(t *testing.T) { // the only way to reliably test this is to verify that the operator didn't attempt to set the ownership field assert.Equal(t, "another-identity", persisted.Labels[v1.LabelOperatedBy]) - - // condition shouldn't be touched when the object belongs to another operator assert.Equal(t, v1.JaegerPhase(""), persisted.Status.Phase) }