diff --git a/api/v1alpha1/argocd_types.go b/api/v1alpha1/argocd_types.go index 02591a901..e4c9efa15 100644 --- a/api/v1alpha1/argocd_types.go +++ b/api/v1alpha1/argocd_types.go @@ -839,6 +839,15 @@ type ArgoCDSpec struct { AggregatedClusterRoles bool `json:"aggregatedClusterRoles,omitempty"` } +const ( + ArgoCDConditionType = "Reconciled" +) + +const ( + ArgoCDConditionReasonSuccess = "Success" + ArgoCDConditionReasonErrorOccurred = "ErrorOccurred" +) + // ArgoCDStatus defines the observed state of ArgoCD // +k8s:openapi-gen=true type ArgoCDStatus struct { @@ -922,6 +931,9 @@ type ArgoCDStatus struct { // Host is the hostname of the Ingress. Host string `json:"host,omitempty"` + + // Conditions is an array of the ArgoCD's status conditions + Conditions []metav1.Condition `json:"conditions,omitempty"` } // Banner defines an additional banner message to be displayed in Argo CD UI diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index e089557e1..b2c9c5ab2 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -35,7 +35,7 @@ func (in *ArgoCD) DeepCopyInto(out *ArgoCD) { out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) in.Spec.DeepCopyInto(&out.Spec) - out.Status = in.Status + in.Status.DeepCopyInto(&out.Status) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ArgoCD. @@ -977,6 +977,13 @@ func (in *ArgoCDSpec) DeepCopy() *ArgoCDSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ArgoCDStatus) DeepCopyInto(out *ArgoCDStatus) { *out = *in + if in.Conditions != nil { + in, out := &in.Conditions, &out.Conditions + *out = make([]metav1.Condition, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ArgoCDStatus. diff --git a/api/v1beta1/argocd_types.go b/api/v1beta1/argocd_types.go index 17cf60652..0821ffed0 100644 --- a/api/v1beta1/argocd_types.go +++ b/api/v1beta1/argocd_types.go @@ -940,6 +940,15 @@ type ArgoCDSpec struct { AggregatedClusterRoles bool `json:"aggregatedClusterRoles,omitempty"` } +const ( + ArgoCDConditionType = "Reconciled" +) + +const ( + ArgoCDConditionReasonSuccess = "Success" + ArgoCDConditionReasonErrorOccurred = "ErrorOccurred" +) + // ArgoCDStatus defines the observed state of ArgoCD // +k8s:openapi-gen=true type ArgoCDStatus struct { @@ -1023,6 +1032,9 @@ type ArgoCDStatus struct { // Host is the hostname of the Ingress. Host string `json:"host,omitempty"` + + // Conditions is an array of the ArgoCD's status conditions + Conditions []metav1.Condition `json:"conditions,omitempty"` } // Banner defines an additional banner message to be displayed in Argo CD UI diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index 36f2e42ad..567bca3c7 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -35,7 +35,7 @@ func (in *ArgoCD) DeepCopyInto(out *ArgoCD) { out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) in.Spec.DeepCopyInto(&out.Spec) - out.Status = in.Status + in.Status.DeepCopyInto(&out.Status) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ArgoCD. @@ -1026,6 +1026,13 @@ func (in *ArgoCDSpec) DeepCopy() *ArgoCDSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ArgoCDStatus) DeepCopyInto(out *ArgoCDStatus) { *out = *in + if in.Conditions != nil { + in, out := &in.Conditions, &out.Conditions + *out = make([]metav1.Condition, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ArgoCDStatus. diff --git a/bundle/manifests/argocd-operator.clusterserviceversion.yaml b/bundle/manifests/argocd-operator.clusterserviceversion.yaml index 41bd4f631..9be3b2803 100644 --- a/bundle/manifests/argocd-operator.clusterserviceversion.yaml +++ b/bundle/manifests/argocd-operator.clusterserviceversion.yaml @@ -247,7 +247,7 @@ metadata: capabilities: Deep Insights categories: Integration & Delivery certified: "false" - createdAt: "2024-12-05T16:51:54Z" + createdAt: "2024-12-16T16:14:14Z" description: Argo CD is a declarative, GitOps continuous delivery tool for Kubernetes. operators.operatorframework.io/builder: operator-sdk-v1.35.0 operators.operatorframework.io/project_layout: go.kubebuilder.io/v4 diff --git a/bundle/manifests/argoproj.io_argocds.yaml b/bundle/manifests/argoproj.io_argocds.yaml index 9dcb71791..cf66d2e72 100644 --- a/bundle/manifests/argoproj.io_argocds.yaml +++ b/bundle/manifests/argoproj.io_argocds.yaml @@ -7165,6 +7165,76 @@ spec: Failed: At least one of the Argo CD applicationSet controller component Pods had a failure. Unknown: The state of the Argo CD applicationSet controller component could not be obtained. type: string + conditions: + description: Conditions is an array of the ArgoCD's status conditions + items: + description: "Condition contains details for one aspect of the current + state of this API Resource.\n---\nThis struct is intended for + direct use as an array at the field path .status.conditions. For + example,\n\n\n\ttype FooStatus struct{\n\t // Represents the + observations of a foo's current state.\n\t // Known .status.conditions.type + are: \"Available\", \"Progressing\", and \"Degraded\"\n\t // + +patchMergeKey=type\n\t // +patchStrategy=merge\n\t // +listType=map\n\t + \ // +listMapKey=type\n\t Conditions []metav1.Condition `json:\"conditions,omitempty\" + patchStrategy:\"merge\" patchMergeKey:\"type\" protobuf:\"bytes,1,rep,name=conditions\"`\n\n\n\t + \ // other fields\n\t}" + properties: + lastTransitionTime: + description: |- + lastTransitionTime is the last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: |- + message is a human readable message indicating details about the transition. + This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: |- + observedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date + with respect to the current state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: |- + reason contains a programmatic identifier indicating the reason for the condition's last transition. + Producers of specific condition types may define expected values and meanings for this field, + and whether the values are considered a guaranteed API. + The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: |- + type of condition in CamelCase or in foo.example.com/CamelCase. + --- + Many .condition.type values are consistent across resources like Available, but because arbitrary conditions can be + useful (see .node.status.conditions), the ability to deconflict is important. + The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt) + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + type: array host: description: Host is the hostname of the Ingress. type: string @@ -24674,6 +24744,76 @@ spec: Failed: At least one of the Argo CD applicationSet controller component Pods had a failure. Unknown: The state of the Argo CD applicationSet controller component could not be obtained. type: string + conditions: + description: Conditions is an array of the ArgoCD's status conditions + items: + description: "Condition contains details for one aspect of the current + state of this API Resource.\n---\nThis struct is intended for + direct use as an array at the field path .status.conditions. For + example,\n\n\n\ttype FooStatus struct{\n\t // Represents the + observations of a foo's current state.\n\t // Known .status.conditions.type + are: \"Available\", \"Progressing\", and \"Degraded\"\n\t // + +patchMergeKey=type\n\t // +patchStrategy=merge\n\t // +listType=map\n\t + \ // +listMapKey=type\n\t Conditions []metav1.Condition `json:\"conditions,omitempty\" + patchStrategy:\"merge\" patchMergeKey:\"type\" protobuf:\"bytes,1,rep,name=conditions\"`\n\n\n\t + \ // other fields\n\t}" + properties: + lastTransitionTime: + description: |- + lastTransitionTime is the last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: |- + message is a human readable message indicating details about the transition. + This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: |- + observedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date + with respect to the current state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: |- + reason contains a programmatic identifier indicating the reason for the condition's last transition. + Producers of specific condition types may define expected values and meanings for this field, + and whether the values are considered a guaranteed API. + The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: |- + type of condition in CamelCase or in foo.example.com/CamelCase. + --- + Many .condition.type values are consistent across resources like Available, but because arbitrary conditions can be + useful (see .node.status.conditions), the ability to deconflict is important. + The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt) + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + type: array host: description: Host is the hostname of the Ingress. type: string diff --git a/config/crd/bases/argoproj.io_argocds.yaml b/config/crd/bases/argoproj.io_argocds.yaml index 4757f39f6..68d5605a2 100644 --- a/config/crd/bases/argoproj.io_argocds.yaml +++ b/config/crd/bases/argoproj.io_argocds.yaml @@ -7154,6 +7154,76 @@ spec: Failed: At least one of the Argo CD applicationSet controller component Pods had a failure. Unknown: The state of the Argo CD applicationSet controller component could not be obtained. type: string + conditions: + description: Conditions is an array of the ArgoCD's status conditions + items: + description: "Condition contains details for one aspect of the current + state of this API Resource.\n---\nThis struct is intended for + direct use as an array at the field path .status.conditions. For + example,\n\n\n\ttype FooStatus struct{\n\t // Represents the + observations of a foo's current state.\n\t // Known .status.conditions.type + are: \"Available\", \"Progressing\", and \"Degraded\"\n\t // + +patchMergeKey=type\n\t // +patchStrategy=merge\n\t // +listType=map\n\t + \ // +listMapKey=type\n\t Conditions []metav1.Condition `json:\"conditions,omitempty\" + patchStrategy:\"merge\" patchMergeKey:\"type\" protobuf:\"bytes,1,rep,name=conditions\"`\n\n\n\t + \ // other fields\n\t}" + properties: + lastTransitionTime: + description: |- + lastTransitionTime is the last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: |- + message is a human readable message indicating details about the transition. + This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: |- + observedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date + with respect to the current state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: |- + reason contains a programmatic identifier indicating the reason for the condition's last transition. + Producers of specific condition types may define expected values and meanings for this field, + and whether the values are considered a guaranteed API. + The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: |- + type of condition in CamelCase or in foo.example.com/CamelCase. + --- + Many .condition.type values are consistent across resources like Available, but because arbitrary conditions can be + useful (see .node.status.conditions), the ability to deconflict is important. + The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt) + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + type: array host: description: Host is the hostname of the Ingress. type: string @@ -24663,6 +24733,76 @@ spec: Failed: At least one of the Argo CD applicationSet controller component Pods had a failure. Unknown: The state of the Argo CD applicationSet controller component could not be obtained. type: string + conditions: + description: Conditions is an array of the ArgoCD's status conditions + items: + description: "Condition contains details for one aspect of the current + state of this API Resource.\n---\nThis struct is intended for + direct use as an array at the field path .status.conditions. For + example,\n\n\n\ttype FooStatus struct{\n\t // Represents the + observations of a foo's current state.\n\t // Known .status.conditions.type + are: \"Available\", \"Progressing\", and \"Degraded\"\n\t // + +patchMergeKey=type\n\t // +patchStrategy=merge\n\t // +listType=map\n\t + \ // +listMapKey=type\n\t Conditions []metav1.Condition `json:\"conditions,omitempty\" + patchStrategy:\"merge\" patchMergeKey:\"type\" protobuf:\"bytes,1,rep,name=conditions\"`\n\n\n\t + \ // other fields\n\t}" + properties: + lastTransitionTime: + description: |- + lastTransitionTime is the last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: |- + message is a human readable message indicating details about the transition. + This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: |- + observedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date + with respect to the current state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: |- + reason contains a programmatic identifier indicating the reason for the condition's last transition. + Producers of specific condition types may define expected values and meanings for this field, + and whether the values are considered a guaranteed API. + The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: |- + type of condition in CamelCase or in foo.example.com/CamelCase. + --- + Many .condition.type values are consistent across resources like Available, but because arbitrary conditions can be + useful (see .node.status.conditions), the ability to deconflict is important. + The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt) + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + type: array host: description: Host is the hostname of the Ingress. type: string diff --git a/controllers/argocd/argocd_controller.go b/controllers/argocd/argocd_controller.go index 32945b57c..a52ac3b78 100644 --- a/controllers/argocd/argocd_controller.go +++ b/controllers/argocd/argocd_controller.go @@ -92,6 +92,23 @@ var ActiveInstanceMap = make(map[string]string) // - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.9.2/pkg/reconcile func (r *ReconcileArgoCD) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.Result, error) { + result, argocd, err := r.internalReconcile(ctx, request) + + message := "" + if err != nil { + message = err.Error() + } + + if error := updateStatusConditionOfArgoCD(ctx, createCondition(message), argocd, r.Client, log); error != nil { + log.Error(error, "unable to update status of ArgoCD") + return reconcile.Result{}, error + } + + return result, err +} + +func (r *ReconcileArgoCD) internalReconcile(ctx context.Context, request ctrl.Request) (ctrl.Result, *argoproj.ArgoCD, error) { + reconcileStartTS := time.Now() defer func() { ReconcileTime.WithLabelValues(request.Namespace).Observe(time.Since(reconcileStartTS).Seconds()) @@ -107,22 +124,24 @@ func (r *ReconcileArgoCD) Reconcile(ctx context.Context, request ctrl.Request) ( // Request object not found, could have been deleted after reconcile request. // Owned objects are automatically garbage collected. For additional cleanup logic use finalizers. // Return and don't requeue - return reconcile.Result{}, nil + return reconcile.Result{}, argocd, nil } // Error reading the object - requeue the request. - return reconcile.Result{}, err + return reconcile.Result{}, argocd, err } // Fetch labelSelector from r.LabelSelector (command-line option) labelSelector, err := labels.Parse(r.LabelSelector) if err != nil { - reqLogger.Info(fmt.Sprintf("error parsing the labelSelector '%s'.", labelSelector)) - return reconcile.Result{}, err + message := fmt.Sprintf("error parsing the labelSelector '%s'.", labelSelector) + reqLogger.Info(message) + return reconcile.Result{}, argocd, fmt.Errorf(message, " error: %w", err) } + // Match the value of labelSelector from ReconcileArgoCD to labels from the argocd instance if !labelSelector.Matches(labels.Set(argocd.Labels)) { reqLogger.Info(fmt.Sprintf("the ArgoCD instance '%s' does not match the label selector '%s' and skipping for reconciliation", request.NamespacedName, r.LabelSelector)) - return reconcile.Result{}, fmt.Errorf("error: failed to reconcile ArgoCD instance: '%s'", request.NamespacedName) + return reconcile.Result{}, argocd, fmt.Errorf("the ArgoCD instance '%s' does not match the label selector '%s' and skipping for reconciliation", request.NamespacedName, r.LabelSelector) } newPhase := argocd.Status.Phase @@ -161,64 +180,65 @@ func (r *ReconcileArgoCD) Reconcile(ctx context.Context, request ctrl.Request) ( if argocd.IsDeletionFinalizerPresent() { if err := r.deleteClusterResources(argocd); err != nil { - return reconcile.Result{}, fmt.Errorf("failed to delete ClusterResources: %w", err) + return reconcile.Result{}, argocd, fmt.Errorf("failed to delete ClusterResources: %w", err) } if isRemoveManagedByLabelOnArgoCDDeletion() { if err := r.removeManagedByLabelFromNamespaces(argocd.Namespace); err != nil { - return reconcile.Result{}, fmt.Errorf("failed to remove label from namespace[%v], error: %w", argocd.Namespace, err) + return reconcile.Result{}, argocd, fmt.Errorf("failed to remove label from namespace[%v], error: %w", argocd.Namespace, err) } } if err := r.removeUnmanagedSourceNamespaceResources(argocd); err != nil { - return reconcile.Result{}, fmt.Errorf("failed to remove resources from sourceNamespaces, error: %w", err) + return reconcile.Result{}, argocd, fmt.Errorf("failed to remove resources from sourceNamespaces, error: %w", err) } if err := r.removeUnmanagedApplicationSetSourceNamespaceResources(argocd); err != nil { - return reconcile.Result{}, fmt.Errorf("failed to remove resources from applicationSetSourceNamespaces, error: %w", err) + return reconcile.Result{}, argocd, fmt.Errorf("failed to remove resources from applicationSetSourceNamespaces, error: %w", err) } if err := r.removeDeletionFinalizer(argocd); err != nil { - return reconcile.Result{}, err + return reconcile.Result{}, argocd, err } // remove namespace of deleted Argo CD instance from deprecationEventEmissionTracker (if exists) so that if another instance // is created in the same namespace in the future, that instance is appropriately tracked delete(DeprecationEventEmissionTracker, argocd.Namespace) } - return reconcile.Result{}, nil + + return reconcile.Result{}, argocd, nil } if !argocd.IsDeletionFinalizerPresent() { if err := r.addDeletionFinalizer(argocd); err != nil { - return reconcile.Result{}, err + return reconcile.Result{}, argocd, err } } // get the latest version of argocd instance before reconciling if err = r.Client.Get(ctx, request.NamespacedName, argocd); err != nil { - return reconcile.Result{}, err + return reconcile.Result{}, argocd, err } if err = r.setManagedNamespaces(argocd); err != nil { - return reconcile.Result{}, err + return reconcile.Result{}, argocd, err } if err = r.setManagedSourceNamespaces(argocd); err != nil { - return reconcile.Result{}, err + return reconcile.Result{}, argocd, err } if err = r.setManagedApplicationSetSourceNamespaces(argocd); err != nil { - return reconcile.Result{}, err + return reconcile.Result{}, argocd, err } if err := r.reconcileResources(argocd); err != nil { // Error reconciling ArgoCD sub-resources - requeue the request. - return reconcile.Result{}, err + return reconcile.Result{}, argocd, err } // Return and don't requeue - return reconcile.Result{}, nil + return reconcile.Result{}, argocd, nil } // SetupWithManager sets up the controller with the Manager. diff --git a/controllers/argocd/argocd_controller_test.go b/controllers/argocd/argocd_controller_test.go index 0b105c65f..64b04945e 100644 --- a/controllers/argocd/argocd_controller_test.go +++ b/controllers/argocd/argocd_controller_test.go @@ -384,3 +384,64 @@ func clusterResources(argocd *argoproj.ArgoCD) []client.Object { newClusterRoleBindingWithname(common.ArgoCDServerComponent, argocd), } } + +func TestReconcileArgoCD_Status_Condition(t *testing.T) { + logf.SetLogger(ZapLogger(true)) + + a := makeTestArgoCD(func(ac *argoproj.ArgoCD) { + ac.Name = "argo-test-2" + ac.Labels = map[string]string{"testfoo": "testbar"} + }) + + resObjs := []client.Object{a} + subresObjs := []client.Object{a} + runtimeObjs := []runtime.Object{} + sch := makeTestReconcilerScheme(argoproj.AddToScheme) + cl := makeTestReconcilerClient(sch, resObjs, subresObjs, runtimeObjs) + rt := makeTestReconciler(cl, sch) + rt.LabelSelector = "foo=bar" + assert.NoError(t, createNamespace(rt, a.Namespace, "")) + + // Instance is not reconciled as the label does not match, error is expected + reqTest := reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: a.Name, + Namespace: a.Namespace, + }, + } + resTest, err := rt.Reconcile(context.TODO(), reqTest) + assert.Error(t, err) + if resTest.Requeue { + t.Fatal("reconcile requeued request") + } + + // Verify condition is updated + assert.NoError(t, rt.Client.Get(context.TODO(), types.NamespacedName{Name: a.Name, Namespace: a.Namespace}, a)) + assert.Equal(t, a.Status.Conditions[0].Type, argoproj.ArgoCDConditionType) + assert.Equal(t, a.Status.Conditions[0].Reason, argoproj.ArgoCDConditionReasonErrorOccurred) + assert.Equal(t, a.Status.Conditions[0].Message, "the ArgoCD instance 'argocd/argo-test-2' does not match the label selector 'foo=bar' and skipping for reconciliation") + assert.Equal(t, a.Status.Conditions[0].Status, metav1.ConditionFalse) + + rt.LabelSelector = "testfoo=testbar" + + // Now instance is reconciled as the label is same, no error is expected + reqTest = reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: a.Name, + Namespace: a.Namespace, + }, + } + resTest, err = rt.Reconcile(context.TODO(), reqTest) + assert.NoError(t, err) + if resTest.Requeue { + t.Fatal("reconcile requeued request") + } + + // Verify condition is updated + assert.NoError(t, rt.Client.Get(context.TODO(), types.NamespacedName{Name: a.Name, Namespace: a.Namespace}, a)) + + assert.Equal(t, a.Status.Conditions[0].Type, argoproj.ArgoCDConditionType) + assert.Equal(t, a.Status.Conditions[0].Reason, argoproj.ArgoCDConditionReasonSuccess) + assert.Equal(t, a.Status.Conditions[0].Message, "") + assert.Equal(t, a.Status.Conditions[0].Status, metav1.ConditionTrue) +} diff --git a/controllers/argocd/keycloak.go b/controllers/argocd/keycloak.go index cd8c4d77f..2ccaa180f 100644 --- a/controllers/argocd/keycloak.go +++ b/controllers/argocd/keycloak.go @@ -998,18 +998,18 @@ func (r *ReconcileArgoCD) updateArgoCDConfiguration(cr *argoproj.ArgoCD, kRouteU err := r.Client.Get(context.TODO(), types.NamespacedName{Name: argoCDSecret.Name, Namespace: argoCDSecret.Namespace}, argoCDSecret) if err != nil { - log.Error(err, fmt.Sprintf("ArgoCD secret not found for ArgoCD %s in namespace %s", - cr.Name, cr.Namespace)) - return err + message := fmt.Sprintf("ArgoCD secret not found for ArgoCD %s in namespace %s", cr.Name, cr.Namespace) + log.Error(err, message) + return fmt.Errorf(message, " error: %w", err) } argoCDSecret.Data["oidc.keycloak.clientSecret"] = []byte(oAuthClientSecret) argoutil.LogResourceUpdate(log, argoCDSecret, "updating client secret for keycloak oidc") err = r.Client.Update(context.TODO(), argoCDSecret) if err != nil { - log.Error(err, fmt.Sprintf("Error updating ArgoCD Secret for ArgoCD %s in namespace %s", - cr.Name, cr.Namespace)) - return err + message := fmt.Sprintf("Error updating ArgoCD Secret for ArgoCD %s in namespace %s", cr.Name, cr.Namespace) + log.Error(err, message) + return fmt.Errorf(message, " error: %w", err) } // Create openshift OAuthClient @@ -1068,38 +1068,36 @@ func (r *ReconcileArgoCD) updateArgoCDConfiguration(cr *argoproj.ArgoCD, kRouteU argoCDCM := newConfigMapWithName(common.ArgoCDConfigMapName, cr) err = r.Client.Get(context.TODO(), types.NamespacedName{Name: argoCDCM.Name, Namespace: argoCDCM.Namespace}, argoCDCM) if err != nil { - log.Error(err, fmt.Sprintf("ArgoCD configmap not found for ArgoCD %s in namespace %s", - cr.Name, cr.Namespace)) - - return err + message := fmt.Sprintf("ArgoCD configmap not found for ArgoCD %s in namespace %s", cr.Name, cr.Namespace) + log.Error(err, message) + return fmt.Errorf(message, " error: %w", err) } argoCDCM.Data[common.ArgoCDKeyOIDCConfig] = string(o) argoutil.LogResourceUpdate(log, argoCDCM, "updating oidc config with keycloak realm") err = r.Client.Update(context.TODO(), argoCDCM) if err != nil { - log.Error(err, fmt.Sprintf("Error updating OIDC Configuration for ArgoCD %s in namespace %s", - cr.Name, cr.Namespace)) - return err + message := fmt.Sprintf("Error updating OIDC Configuration for ArgoCD %s in namespace %s", cr.Name, cr.Namespace) + log.Error(err, message) + return fmt.Errorf(message, " error: %w", err) } // Update RBAC for ArgoCD Instance. argoRBACCM := newConfigMapWithName(common.ArgoCDRBACConfigMapName, cr) err = r.Client.Get(context.TODO(), types.NamespacedName{Name: argoRBACCM.Name, Namespace: argoRBACCM.Namespace}, argoRBACCM) if err != nil { - log.Error(err, fmt.Sprintf("ArgoCD RBAC configmap not found for ArgoCD %s in namespace %s", - cr.Name, cr.Namespace)) - - return err + message := fmt.Sprintf("ArgoCD RBAC configmap not found for ArgoCD %s in namespace %s", cr.Name, cr.Namespace) + log.Error(err, message) + return fmt.Errorf(message, " error: %w", err) } argoRBACCM.Data["scopes"] = "[groups,email]" argoutil.LogResourceUpdate(log, argoRBACCM, "updating rbac scopes for keycloak") err = r.Client.Update(context.TODO(), argoRBACCM) if err != nil { - log.Error(err, fmt.Sprintf("Error updating ArgoCD RBAC configmap %s in namespace %s", - cr.Name, cr.Namespace)) - return err + message := fmt.Sprintf("Error updating ArgoCD RBAC configmap %s in namespace %s", cr.Name, cr.Namespace) + log.Error(err, message) + return fmt.Errorf(message, " error: %w", err) } return nil @@ -1116,9 +1114,9 @@ func handleKeycloakPodDeletion(dc *appsv1.DeploymentConfig) error { // Initialize deployment config client. dcClient, err := oappsv1client.NewForConfig(cfg) if err != nil { - log.Error(err, fmt.Sprintf("unable to create apps client for Deployment config %s in namespace %s", - dc.Name, dc.Namespace)) - return err + message := fmt.Sprintf("unable to create apps client for Deployment config %s in namespace %s", dc.Name, dc.Namespace) + log.Error(err, message) + return fmt.Errorf(message, " error: %w", message) } existingDC, err := dcClient.DeploymentConfigs(dc.Namespace).Get(context.TODO(), defaultKeycloakIdentifier, metav1.GetOptions{}) @@ -1175,17 +1173,17 @@ func deleteKeycloakConfiguration(cr *argoproj.ArgoCD) error { func deleteKeycloakConfigForOpenShift(cr *argoproj.ArgoCD) error { cfg, err := config.GetConfig() if err != nil { - log.Error(err, fmt.Sprintf("unable to get k8s config for ArgoCD %s in namespace %s", - cr.Name, cr.Namespace)) - return err + message := fmt.Sprintf("unable to get k8s config for ArgoCD %s in namespace %s", cr.Name, cr.Namespace) + log.Error(err, message) + return fmt.Errorf(message, " error: %w", err) } // Initialize template client. templateclient, err := templatev1client.NewForConfig(cfg) if err != nil { - log.Error(err, fmt.Sprintf("unable to create Template client for ArgoCD %s in namespace %s", - cr.Name, cr.Namespace)) - return err + message := fmt.Sprintf("unable to create Template client for ArgoCD %s in namespace %s", cr.Name, cr.Namespace) + log.Error(err, message) + return fmt.Errorf(message, " error: %w", err) } log.Info(fmt.Sprintf("Delete Template Instance for ArgoCD %s in namespace %s", @@ -1214,9 +1212,9 @@ func deleteOAuthClient(cr *argoproj.ArgoCD) error { cfg, err := config.GetConfig() if err != nil { - log.Error(err, fmt.Sprintf("unable to get k8s config for ArgoCD %s in namespace %s", - cr.Name, cr.Namespace)) - return err + message := fmt.Sprintf("unable to get k8s config for ArgoCD %s in namespace %s", cr.Name, cr.Namespace) + log.Error(err, message) + return fmt.Errorf(message, " error: %w", err) } // We use the foreground propagation policy to ensure that the garbage @@ -1228,9 +1226,9 @@ func deleteOAuthClient(cr *argoproj.ArgoCD) error { // Delete OAuthClient created for keycloak. oauth, err := oauthclient.NewForConfig(cfg) if err != nil { - log.Error(err, fmt.Sprintf("unable to create oAuth client for ArgoCD %s in namespace %s", - cr.Name, cr.Namespace)) - return err + message := fmt.Sprintf("unable to create oAuth client for ArgoCD %s in namespace %s", cr.Name, cr.Namespace) + log.Error(err, message) + return fmt.Errorf(message, " error: %w", err) } log.Info(fmt.Sprintf("Delete OAuthClient for ArgoCD %s in namespace %s", cr.Name, cr.Namespace)) @@ -1257,9 +1255,9 @@ func deleteKeycloakConfigForK8s(cr *argoproj.ArgoCD) error { cfg, err := config.GetConfig() if err != nil { - log.Error(err, fmt.Sprintf("unable to get k8s config for ArgoCD %s in namespace %s", - cr.Name, cr.Namespace)) - return err + message := fmt.Sprintf("unable to get k8s config for ArgoCD %s in namespace %s", cr.Name, cr.Namespace) + log.Error(err, message) + return fmt.Errorf(message, " error: %w", err) } clientset, err := kubernetes.NewForConfig(cfg) @@ -1387,9 +1385,9 @@ func (r *ReconcileArgoCD) reconcileKeycloakForOpenShift(cr *argoproj.ArgoCD) err // Create a keycloak realm and publish. response, err := createRealm(cfg) if err != nil { - log.Error(err, fmt.Sprintf("Failed posting keycloak realm configuration for ArgoCD %s in namespace %s", - cr.Name, cr.Namespace)) - return err + message := fmt.Sprintf("Failed posting keycloak realm configuration for ArgoCD %s in namespace %s", cr.Name, cr.Namespace) + log.Error(err, message) + return fmt.Errorf(message, " error: %w", err) } if response == successResponse { @@ -1428,9 +1426,9 @@ func (r *ReconcileArgoCD) reconcileKeycloakForOpenShift(cr *argoproj.ArgoCD) err // or when user requests to update the OIDC configuration through `.spec.sso.keycloak.rootCA`. err = r.updateArgoCDConfiguration(cr, keycloakRouteURL) if err != nil { - log.Error(err, fmt.Sprintf("Failed to update OIDC Configuration for ArgoCD %s in namespace %s", - cr.Name, cr.Namespace)) - return err + message := fmt.Sprintf("Failed to update OIDC Configuration for ArgoCD %s in namespace %s", cr.Name, cr.Namespace) + log.Error(err, message) + return fmt.Errorf(message, " error: %w", err) } } @@ -1442,9 +1440,9 @@ func (r *ReconcileArgoCD) reconcileKeycloak(cr *argoproj.ArgoCD) error { err := r.newKeycloakInstance(cr) if err != nil { - log.Error(err, fmt.Sprintf("Failed creating keycloak instance for ArgoCD %s in Namespace %s", - cr.Name, cr.Namespace)) - return err + message := fmt.Sprintf("Failed creating keycloak instance for ArgoCD %s in Namespace %s", cr.Name, cr.Namespace) + log.Error(err, message) + return fmt.Errorf(message, " error: %w", err) } existingDeployment := &k8sappsv1.Deployment{ @@ -1507,9 +1505,9 @@ func (r *ReconcileArgoCD) reconcileKeycloak(cr *argoproj.ArgoCD) error { // Create a keycloak realm and publish. response, err := createRealm(cfg) if err != nil { - log.Error(err, fmt.Sprintf("Failed posting keycloak realm configuration for ArgoCD %s in namespace %s", - cr.Name, cr.Namespace)) - return err + message := fmt.Sprintf("Failed posting keycloak realm configuration for ArgoCD %s in namespace %s", cr.Name, cr.Namespace) + log.Error(err, message) + return fmt.Errorf(message, " error: %w", err) } if response == successResponse { @@ -1529,9 +1527,9 @@ func (r *ReconcileArgoCD) reconcileKeycloak(cr *argoproj.ArgoCD) error { // or when user requests to update the OIDC configuration through `.spec.sso.keycloak.rootCA`. err = r.updateArgoCDConfiguration(cr, kIngURL) if err != nil { - log.Error(err, fmt.Sprintf("Failed to update OIDC Configuration for ArgoCD %s in namespace %s", - cr.Name, cr.Namespace)) - return err + message := fmt.Sprintf("Failed to update OIDC Configuration for ArgoCD %s in namespace %s", cr.Name, cr.Namespace) + log.Error(err, message) + return fmt.Errorf(message, " error: %w", err) } } diff --git a/controllers/argocd/keycloak_client.go b/controllers/argocd/keycloak_client.go index f7f2782c3..f6c81366f 100644 --- a/controllers/argocd/keycloak_client.go +++ b/controllers/argocd/keycloak_client.go @@ -185,7 +185,7 @@ func (h *httpclient) validateKeycloakURL(URL string) error { res, err := h.requester.Do(req) if err != nil { log.Info("Cannot access keycloak with Internal service name, trying keycloak Route URL") - return err + return fmt.Errorf("Cannot access keycloak with Internal service name, trying keycloak Route URL. error: %w", err) } _ = res.Body.Close() return nil diff --git a/controllers/argocd/networkpolicies.go b/controllers/argocd/networkpolicies.go index 311b612f1..0cd64f0c6 100644 --- a/controllers/argocd/networkpolicies.go +++ b/controllers/argocd/networkpolicies.go @@ -136,7 +136,7 @@ func (r *ReconcileArgoCD) ReconcileRedisNetworkPolicy(cr *argoproj.ArgoCD) error err := r.Client.Update(context.TODO(), existing) if err != nil { log.Error(err, "Failed to update redis network policy") - return err + return fmt.Errorf("Failed to update redis network policy. error: %w", err) } } @@ -148,14 +148,14 @@ func (r *ReconcileArgoCD) ReconcileRedisNetworkPolicy(cr *argoproj.ArgoCD) error // Set the ArgoCD instance as the owner and controller if err := controllerutil.SetControllerReference(cr, networkPolicy, r.Scheme); err != nil { log.Error(err, "Failed to set controller reference on redis network policy") - return err + return fmt.Errorf("Failed to set controller reference on redis network policy. error: %w", err) } argoutil.LogResourceCreation(log, networkPolicy) err := r.Client.Create(context.TODO(), networkPolicy) if err != nil { log.Error(err, "Failed to create redis network policy") - return err + return fmt.Errorf("Failed to create redis network policy. error: %w", err) } return nil @@ -258,7 +258,7 @@ func (r *ReconcileArgoCD) ReconcileRedisHANetworkPolicy(cr *argoproj.ArgoCD) err err := r.Client.Update(context.TODO(), existing) if err != nil { log.Error(err, "Failed to update redis ha network policy") - return err + return fmt.Errorf("Failed to update redis ha network policy. error: %w", err) } } @@ -270,14 +270,14 @@ func (r *ReconcileArgoCD) ReconcileRedisHANetworkPolicy(cr *argoproj.ArgoCD) err // Set the ArgoCD instance as the owner and controller if err := controllerutil.SetControllerReference(cr, networkPolicy, r.Scheme); err != nil { log.Error(err, "Failed to set controller reference on redis ha network policy") - return err + return fmt.Errorf("Failed to set controller reference on redis ha network policy. error: %w", err) } argoutil.LogResourceCreation(log, networkPolicy) err := r.Client.Create(context.TODO(), networkPolicy) if err != nil { log.Error(err, "Failed to create redis ha network policy") - return err + return fmt.Errorf("Failed to create redis ha network policy. error: %w", err) } return nil diff --git a/controllers/argocd/sso.go b/controllers/argocd/sso.go index fa917d848..6e8f9186f 100644 --- a/controllers/argocd/sso.go +++ b/controllers/argocd/sso.go @@ -176,7 +176,7 @@ func (r *ReconcileArgoCD) reconcileSSO(cr *argoproj.ArgoCD) error { // Trigger reconciliation of any Dex resources so they get deleted if err := r.reconcileDexResources(cr); err != nil && !apiErrors.IsNotFound(err) { log.Error(err, "Unable to delete existing dex resources before configuring keycloak") - return err + return fmt.Errorf("Unable to delete existing dex resources before configuring keycloak. error: %w", err) } if err := r.reconcileKeycloakConfiguration(cr); err != nil { @@ -187,7 +187,7 @@ func (r *ReconcileArgoCD) reconcileSSO(cr *argoproj.ArgoCD) error { // Delete any lingering keycloak artifacts before Dex is configured as this is not handled by the reconcilliation loop if err := deleteKeycloakConfiguration(cr); err != nil && !apiErrors.IsNotFound(err) { log.Error(err, "Unable to delete existing SSO configuration before configuring Dex") - return err + return fmt.Errorf("Unable to delete existing SSO configuration before configuring Dex. error: %w", err) } if err := r.reconcileDexResources(cr); err != nil { @@ -207,13 +207,13 @@ func (r *ReconcileArgoCD) deleteSSOConfiguration(newCr *argoproj.ArgoCD, oldCr * if oldCr.Spec.SSO.Provider.ToLower() == argoproj.SSOProviderTypeKeycloak { if err := deleteKeycloakConfiguration(newCr); err != nil { log.Error(err, "Unable to delete existing keycloak configuration") - return err + return fmt.Errorf("Unable to delete existing keycloak configuration. error: %w", err) } } else if oldCr.Spec.SSO.Provider.ToLower() == argoproj.SSOProviderTypeDex { // Trigger reconciliation of Dex resources so they get deleted if err := r.deleteDexResources(newCr); err != nil { log.Error(err, "Unable to reconcile necessary resources for uninstallation of Dex") - return err + return fmt.Errorf("Unable to reconcile necessary resources for uninstallation of Dex. error: %w", err) } } diff --git a/controllers/argocd/util.go b/controllers/argocd/util.go index 160b12c2d..052496e02 100644 --- a/controllers/argocd/util.go +++ b/controllers/argocd/util.go @@ -32,6 +32,7 @@ import ( "gopkg.in/yaml.v2" "github.com/argoproj/argo-cd/v2/util/glob" + "github.com/go-logr/logr" "github.com/argoproj-labs/argocd-operator/api/v1alpha1" argoproj "github.com/argoproj-labs/argocd-operator/api/v1beta1" @@ -624,14 +625,14 @@ func loadTemplateFile(path string, params map[string]string) (string, error) { tmpl, err := template.ParseFiles(path) if err != nil { log.Error(err, "unable to parse template") - return "", err + return "", fmt.Errorf("unable to parse template. error: %w", err) } buf := new(bytes.Buffer) err = tmpl.Execute(buf, params) if err != nil { log.Error(err, "unable to execute template") - return "", err + return "", fmt.Errorf("unable to execute template. error: %w", err) } return buf.String(), nil } @@ -1299,8 +1300,9 @@ func deleteRBACsForNamespace(sourceNS string, k8sClient kubernetes.Interface) er labelSelector := metav1.LabelSelector{MatchLabels: map[string]string{common.ArgoCDKeyPartOf: common.ArgoCDAppName}} roles, err := k8sClient.RbacV1().Roles(sourceNS).List(context.TODO(), metav1.ListOptions{LabelSelector: labels.Set(labelSelector.MatchLabels).String()}) if err != nil { - log.Error(err, fmt.Sprintf("failed to list roles for namespace: %s", sourceNS)) - return err + message := fmt.Sprintf("failed to list roles for namespace: %s", sourceNS) + log.Error(err, message) + return fmt.Errorf(message, " error: %w", err) } // Delete all the retrieved roles @@ -1315,8 +1317,9 @@ func deleteRBACsForNamespace(sourceNS string, k8sClient kubernetes.Interface) er // List all the roles bindings created for ArgoCD using the label selector roleBindings, err := k8sClient.RbacV1().RoleBindings(sourceNS).List(context.TODO(), metav1.ListOptions{LabelSelector: labels.Set(labelSelector.MatchLabels).String()}) if err != nil { - log.Error(err, fmt.Sprintf("failed to list role bindings for namespace: %s", sourceNS)) - return err + message := fmt.Sprintf("failed to list role bindings for namespace: %s", sourceNS) + log.Error(err, message) + return fmt.Errorf(message, " error: %w", err) } // Delete all the retrieved role bindings @@ -1337,8 +1340,9 @@ func deleteManagedNamespaceFromClusterSecret(ownerNS, sourceNS string, k8sClient labelSelector := metav1.LabelSelector{MatchLabels: map[string]string{common.ArgoCDSecretTypeLabel: "cluster"}} secrets, err := k8sClient.CoreV1().Secrets(ownerNS).List(context.TODO(), metav1.ListOptions{LabelSelector: labels.Set(labelSelector.MatchLabels).String()}) if err != nil { - log.Error(err, fmt.Sprintf("failed to retrieve secrets for namespace: %s", ownerNS)) - return err + message := fmt.Sprintf("failed to retrieve secrets for namespace: %s", ownerNS) + log.Error(err, message) + return fmt.Errorf(message, " error: %w", err) } for _, secret := range secrets.Items { if string(secret.Data["server"]) != common.ArgoCDDefaultServer { @@ -1360,8 +1364,9 @@ func deleteManagedNamespaceFromClusterSecret(ownerNS, sourceNS string, k8sClient // Update the secret with the updated list of namespaces argoutil.LogResourceUpdate(log, &secret, "removing managed namespace", sourceNS) if _, err = k8sClient.CoreV1().Secrets(ownerNS).Update(context.TODO(), &secret, metav1.UpdateOptions{}); err != nil { - log.Error(err, fmt.Sprintf("failed to update cluster permission secret for namespace: %s", ownerNS)) - return err + message := fmt.Sprintf("failed to update cluster permission secret for namespace: %s", ownerNS) + log.Error(err, message) + return fmt.Errorf(message, " error: %w", err) } } } @@ -1722,3 +1727,79 @@ func addKubernetesData(source map[string]string, live map[string]string) { } } } + +// updateStatusConditionOfArgoCD calls Set Condition of ArgoCD status +func updateStatusConditionOfArgoCD(ctx context.Context, condition metav1.Condition, cr *argoproj.ArgoCD, k8sClient client.Client, log logr.Logger) error { + changed, newConditions := insertOrUpdateConditionsInSlice(condition, cr.Status.Conditions) + + if changed { + // get the latest version of argocd instance before updating + if err := k8sClient.Get(ctx, types.NamespacedName{Name: cr.Name, Namespace: cr.Namespace}, cr); err != nil { + return err + } + + cr.Status.Conditions = newConditions + + if err := k8sClient.Status().Update(ctx, cr); err != nil { + log.Error(err, "unable to update RolloutManager status condition") + return err + } + } + return nil +} + +// insertOrUpdateConditionsInSlice is a generic function for inserting/updating metav1.Condition into a slice of []metav1.Condition +func insertOrUpdateConditionsInSlice(newCondition metav1.Condition, existingConditions []metav1.Condition) (bool, []metav1.Condition) { + + // Check if condition with same type is already set, if Yes then check if content is same, + // If content is not same update LastTransitionTime + index := -1 + for i, Condition := range existingConditions { + if Condition.Type == newCondition.Type { + index = i + break + } + } + + now := metav1.Now() + + changed := false + + if index == -1 { + newCondition.LastTransitionTime = now + existingConditions = append(existingConditions, newCondition) + changed = true + + } else if existingConditions[index].Message != newCondition.Message || + existingConditions[index].Reason != newCondition.Reason || + existingConditions[index].Status != newCondition.Status { + + newCondition.LastTransitionTime = now + existingConditions[index] = newCondition + changed = true + } + + return changed, existingConditions + +} + +// createCondition returns Condition based on input provided. +// 1. Returns Success condition if no error message is provided, all fields are default. +// 2. If Message is provided, it returns Failed condition having all default fields except Message. +func createCondition(message string) metav1.Condition { + if message == "" { + return metav1.Condition{ + Type: argoproj.ArgoCDConditionType, + Reason: argoproj.ArgoCDConditionReasonSuccess, + Message: "", + Status: metav1.ConditionTrue, + } + } + + return metav1.Condition{ + Type: argoproj.ArgoCDConditionType, + Reason: argoproj.ArgoCDConditionReasonErrorOccurred, + Message: message, + Status: metav1.ConditionFalse, + } +} diff --git a/controllers/argocd/util_test.go b/controllers/argocd/util_test.go index c1b9c3b0c..c676764b9 100644 --- a/controllers/argocd/util_test.go +++ b/controllers/argocd/util_test.go @@ -1173,3 +1173,156 @@ func TestRetainKubernetesData(t *testing.T) { }) } } + +func TestUpdateStatusConditionOfArgoCD_Success(t *testing.T) { + logf.SetLogger(ZapLogger(true)) + ctx := context.Background() + a := makeTestArgoCD(deletedAt(time.Now())) + resObjs := []client.Object{a} + subresObjs := []client.Object{a} + runtimeObjs := []runtime.Object{} + sch := makeTestReconcilerScheme(argoproj.AddToScheme) + cl := makeTestReconcilerClient(sch, resObjs, subresObjs, runtimeObjs) + r := makeTestReconciler(cl, sch) + + argocd := argoproj.ArgoCD{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-rm-1", + Namespace: "test-ns-1", + }, + } + + assert.NoError(t, createNamespace(r, argocd.Namespace, "")) + assert.NoError(t, r.Client.Create(ctx, &argocd)) + assert.NoError(t, updateStatusConditionOfArgoCD(ctx, createCondition(""), &argocd, r.Client, log)) + + assert.Equal(t, argocd.Status.Conditions[0].Type, argoproj.ArgoCDConditionType) + assert.Equal(t, argocd.Status.Conditions[0].Reason, argoproj.ArgoCDConditionReasonSuccess) + assert.Equal(t, argocd.Status.Conditions[0].Message, "") + assert.Equal(t, argocd.Status.Conditions[0].Status, metav1.ConditionTrue) +} + +func TestUpdateStatusConditionOfArgoCD_Fail(t *testing.T) { + logf.SetLogger(ZapLogger(true)) + ctx := context.Background() + a := makeTestArgoCD(deletedAt(time.Now())) + resObjs := []client.Object{a} + subresObjs := []client.Object{a} + runtimeObjs := []runtime.Object{} + sch := makeTestReconcilerScheme(argoproj.AddToScheme) + cl := makeTestReconcilerClient(sch, resObjs, subresObjs, runtimeObjs) + r := makeTestReconciler(cl, sch) + + argocd := argoproj.ArgoCD{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-rm-1", + Namespace: "test-ns-1", + }, + } + + assert.NoError(t, createNamespace(r, argocd.Namespace, "")) + assert.NoError(t, r.Client.Create(ctx, &argocd)) + assert.NoError(t, updateStatusConditionOfArgoCD(ctx, createCondition("some error"), &argocd, r.Client, log)) + + assert.Equal(t, argocd.Status.Conditions[0].Type, argoproj.ArgoCDConditionType) + assert.Equal(t, argocd.Status.Conditions[0].Reason, argoproj.ArgoCDConditionReasonErrorOccurred) + assert.Equal(t, argocd.Status.Conditions[0].Message, "some error") + assert.Equal(t, argocd.Status.Conditions[0].Status, metav1.ConditionFalse) + + // Update error condition + assert.NoError(t, updateStatusConditionOfArgoCD(ctx, createCondition("some other error"), &argocd, r.Client, log)) + + assert.Equal(t, argocd.Status.Conditions[0].Type, argoproj.ArgoCDConditionType) + assert.Equal(t, argocd.Status.Conditions[0].Reason, argoproj.ArgoCDConditionReasonErrorOccurred) + assert.Equal(t, argocd.Status.Conditions[0].Message, "some other error") + assert.Equal(t, argocd.Status.Conditions[0].Status, metav1.ConditionFalse) + + // Update success condition + assert.NoError(t, updateStatusConditionOfArgoCD(ctx, createCondition(""), &argocd, r.Client, log)) + + assert.Equal(t, argocd.Status.Conditions[0].Type, argoproj.ArgoCDConditionType) + assert.Equal(t, argocd.Status.Conditions[0].Reason, argoproj.ArgoCDConditionReasonSuccess) + assert.Equal(t, argocd.Status.Conditions[0].Message, "") + assert.Equal(t, argocd.Status.Conditions[0].Status, metav1.ConditionTrue) +} + +func TestInsertOrUpdateConditionsInSlice_add_new_condition(t *testing.T) { + logf.SetLogger(ZapLogger(true)) + existingConditions := []metav1.Condition{} + newCondition := metav1.Condition{ + Type: argoproj.ArgoCDConditionType, + Status: metav1.ConditionTrue, + Reason: "test reason", + Message: "test message", + } + changed, conditions := insertOrUpdateConditionsInSlice(newCondition, existingConditions) + + assert.True(t, changed) + assert.Len(t, conditions, 1) + assert.Equal(t, conditions[0].Type, newCondition.Type) + assert.Equal(t, conditions[0].Status, newCondition.Status) + assert.Equal(t, conditions[0].Reason, newCondition.Reason) + assert.Equal(t, conditions[0].Message, newCondition.Message) +} + +func TestInsertOrUpdateConditionsInSlice_change_existing_condition(t *testing.T) { + logf.SetLogger(ZapLogger(true)) + existingConditions := []metav1.Condition{ + { + Type: argoproj.ArgoCDConditionType, + Status: metav1.ConditionTrue, + Reason: "test reason", + Message: "test message", + }, + } + newCondition := metav1.Condition{ + Type: argoproj.ArgoCDConditionType, + Status: metav1.ConditionFalse, + Reason: "Updated test reason", + Message: "Updated test message", + } + + changed, conditions := insertOrUpdateConditionsInSlice(newCondition, existingConditions) + + assert.True(t, changed) + assert.Len(t, conditions, 1) + assert.Equal(t, conditions[0].Type, newCondition.Type) + assert.Equal(t, conditions[0].Status, newCondition.Status) + assert.Equal(t, conditions[0].Reason, newCondition.Reason) + assert.Equal(t, conditions[0].Message, newCondition.Message) +} + +func TestInsertOrUpdateConditionsInSlice_add_another_condition(t *testing.T) { + logf.SetLogger(ZapLogger(true)) + newCondition := metav1.Condition{ + Type: argoproj.ArgoCDConditionType, + Status: metav1.ConditionTrue, + Reason: "test reason", + Message: "test message", + } + unrelatedCondition := metav1.Condition{ + Type: "UnrelatedCondition", + Status: metav1.ConditionFalse, + Reason: "some reason", + Message: "some message", + } + existingConditions := []metav1.Condition{ + unrelatedCondition, + } + + changed, conditions := insertOrUpdateConditionsInSlice(newCondition, existingConditions) + assert.True(t, changed) + assert.Len(t, conditions, 2) + + //Check that the unrelated condition is still present + assert.Equal(t, conditions[0].Type, unrelatedCondition.Type) + assert.Equal(t, conditions[0].Status, unrelatedCondition.Status) + assert.Equal(t, conditions[0].Reason, unrelatedCondition.Reason) + assert.Equal(t, conditions[0].Message, unrelatedCondition.Message) + + //Check that the new condition was added + assert.Equal(t, conditions[1].Type, newCondition.Type) + assert.Equal(t, conditions[1].Status, newCondition.Status) + assert.Equal(t, conditions[1].Reason, newCondition.Reason) + assert.Equal(t, conditions[1].Message, newCondition.Message) +} diff --git a/controllers/argoutil/api.go b/controllers/argoutil/api.go index 946cc1f48..872c3dc50 100644 --- a/controllers/argoutil/api.go +++ b/controllers/argoutil/api.go @@ -68,7 +68,7 @@ func IsAPIRegistered(group string, version string) (bool, error) { client, err := aggregator.NewForConfig(cfg) if err != nil { log.Error(err, "unable to create a kube-aggregator client") - return false, err + return false, fmt.Errorf("unable to create a kube-aggregator client. error: %w", err) } _, err = client.ApiregistrationV1().APIServices(). @@ -78,8 +78,9 @@ func IsAPIRegistered(group string, version string) (bool, error) { log.Info(fmt.Sprintf("%s/%s API is not registered", group, version)) return false, nil } else { - log.Error(err, fmt.Sprintf("%s/%s API registration check failed.", group, version)) - return false, err + message := fmt.Sprintf("%s/%s API registration check failed.", group, version) + log.Error(err, message) + return false, fmt.Errorf(message, " error: %w", err) } } log.Info(fmt.Sprintf("%s/%s API is registered", group, version)) diff --git a/deploy/olm-catalog/argocd-operator/0.13.0/argocd-operator.v0.13.0.clusterserviceversion.yaml b/deploy/olm-catalog/argocd-operator/0.13.0/argocd-operator.v0.13.0.clusterserviceversion.yaml index 41bd4f631..9be3b2803 100644 --- a/deploy/olm-catalog/argocd-operator/0.13.0/argocd-operator.v0.13.0.clusterserviceversion.yaml +++ b/deploy/olm-catalog/argocd-operator/0.13.0/argocd-operator.v0.13.0.clusterserviceversion.yaml @@ -247,7 +247,7 @@ metadata: capabilities: Deep Insights categories: Integration & Delivery certified: "false" - createdAt: "2024-12-05T16:51:54Z" + createdAt: "2024-12-16T16:14:14Z" description: Argo CD is a declarative, GitOps continuous delivery tool for Kubernetes. operators.operatorframework.io/builder: operator-sdk-v1.35.0 operators.operatorframework.io/project_layout: go.kubebuilder.io/v4 diff --git a/deploy/olm-catalog/argocd-operator/0.13.0/argoproj.io_argocds.yaml b/deploy/olm-catalog/argocd-operator/0.13.0/argoproj.io_argocds.yaml index 9dcb71791..cf66d2e72 100644 --- a/deploy/olm-catalog/argocd-operator/0.13.0/argoproj.io_argocds.yaml +++ b/deploy/olm-catalog/argocd-operator/0.13.0/argoproj.io_argocds.yaml @@ -7165,6 +7165,76 @@ spec: Failed: At least one of the Argo CD applicationSet controller component Pods had a failure. Unknown: The state of the Argo CD applicationSet controller component could not be obtained. type: string + conditions: + description: Conditions is an array of the ArgoCD's status conditions + items: + description: "Condition contains details for one aspect of the current + state of this API Resource.\n---\nThis struct is intended for + direct use as an array at the field path .status.conditions. For + example,\n\n\n\ttype FooStatus struct{\n\t // Represents the + observations of a foo's current state.\n\t // Known .status.conditions.type + are: \"Available\", \"Progressing\", and \"Degraded\"\n\t // + +patchMergeKey=type\n\t // +patchStrategy=merge\n\t // +listType=map\n\t + \ // +listMapKey=type\n\t Conditions []metav1.Condition `json:\"conditions,omitempty\" + patchStrategy:\"merge\" patchMergeKey:\"type\" protobuf:\"bytes,1,rep,name=conditions\"`\n\n\n\t + \ // other fields\n\t}" + properties: + lastTransitionTime: + description: |- + lastTransitionTime is the last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: |- + message is a human readable message indicating details about the transition. + This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: |- + observedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date + with respect to the current state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: |- + reason contains a programmatic identifier indicating the reason for the condition's last transition. + Producers of specific condition types may define expected values and meanings for this field, + and whether the values are considered a guaranteed API. + The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: |- + type of condition in CamelCase or in foo.example.com/CamelCase. + --- + Many .condition.type values are consistent across resources like Available, but because arbitrary conditions can be + useful (see .node.status.conditions), the ability to deconflict is important. + The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt) + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + type: array host: description: Host is the hostname of the Ingress. type: string @@ -24674,6 +24744,76 @@ spec: Failed: At least one of the Argo CD applicationSet controller component Pods had a failure. Unknown: The state of the Argo CD applicationSet controller component could not be obtained. type: string + conditions: + description: Conditions is an array of the ArgoCD's status conditions + items: + description: "Condition contains details for one aspect of the current + state of this API Resource.\n---\nThis struct is intended for + direct use as an array at the field path .status.conditions. For + example,\n\n\n\ttype FooStatus struct{\n\t // Represents the + observations of a foo's current state.\n\t // Known .status.conditions.type + are: \"Available\", \"Progressing\", and \"Degraded\"\n\t // + +patchMergeKey=type\n\t // +patchStrategy=merge\n\t // +listType=map\n\t + \ // +listMapKey=type\n\t Conditions []metav1.Condition `json:\"conditions,omitempty\" + patchStrategy:\"merge\" patchMergeKey:\"type\" protobuf:\"bytes,1,rep,name=conditions\"`\n\n\n\t + \ // other fields\n\t}" + properties: + lastTransitionTime: + description: |- + lastTransitionTime is the last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: |- + message is a human readable message indicating details about the transition. + This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: |- + observedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date + with respect to the current state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: |- + reason contains a programmatic identifier indicating the reason for the condition's last transition. + Producers of specific condition types may define expected values and meanings for this field, + and whether the values are considered a guaranteed API. + The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: |- + type of condition in CamelCase or in foo.example.com/CamelCase. + --- + Many .condition.type values are consistent across resources like Available, but because arbitrary conditions can be + useful (see .node.status.conditions), the ability to deconflict is important. + The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt) + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + type: array host: description: Host is the hostname of the Ingress. type: string diff --git a/tests/k8s/1-001_validate_basic/01-assert.yaml b/tests/k8s/1-001_validate_basic/01-assert.yaml index c375020fc..bc7082a83 100644 --- a/tests/k8s/1-001_validate_basic/01-assert.yaml +++ b/tests/k8s/1-001_validate_basic/01-assert.yaml @@ -10,6 +10,11 @@ metadata: name: example-argocd status: phase: Available + conditions: + - message: "" + reason: Success + status: "True" + type: Reconciled --- apiVersion: apps/v1 kind: StatefulSet