From 271cee786f49f3de41a065bcfd46a7f3e766710b Mon Sep 17 00:00:00 2001 From: Bryce Palmer Date: Thu, 30 Mar 2023 10:40:09 -0400 Subject: [PATCH 1/6] communicate CatalogSource ready status Signed-off-by: Bryce Palmer --- .../core.catalogd.io_catalogsources.yaml | 74 ++++++++++++++++++- pkg/apis/core/v1beta1/catalogsource_types.go | 12 ++- .../core/v1beta1/zz_generated.deepcopy.go | 7 ++ .../core/catalogsource_controller.go | 22 +++++- 4 files changed, 111 insertions(+), 4 deletions(-) diff --git a/config/crd/bases/core.catalogd.io_catalogsources.yaml b/config/crd/bases/core.catalogd.io_catalogsources.yaml index 873675d9..f3310075 100644 --- a/config/crd/bases/core.catalogd.io_catalogsources.yaml +++ b/config/crd/bases/core.catalogd.io_catalogsources.yaml @@ -52,14 +52,84 @@ spec: status: description: CatalogSourceStatus defines the observed state of CatalogSource properties: + conditions: + description: Conditions store the status conditions of the CatalogSource + instances + items: + description: "Condition contains details for one aspect of the current + state of this API Resource. --- This struct is intended for direct + use as an array at the field path .status.conditions. For example, + \n type FooStatus struct{ // Represents the observations of a + foo's current state. // Known .status.conditions.type are: \"Available\", + \"Progressing\", and \"Degraded\" // +patchMergeKey=type // +patchStrategy=merge + // +listType=map // +listMapKey=type Conditions []metav1.Condition + `json:\"conditions,omitempty\" patchStrategy:\"merge\" patchMergeKey:\"type\" + protobuf:\"bytes,1,rep,name=conditions\"` \n // other fields }" + 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 latestImagePoll: description: The last time the image has been polled to ensure the image is up-to-date format: date-time type: string - required: - - latestImagePoll type: object type: object served: true storage: true + subresources: + status: {} diff --git a/pkg/apis/core/v1beta1/catalogsource_types.go b/pkg/apis/core/v1beta1/catalogsource_types.go index 59fe84af..204efb31 100644 --- a/pkg/apis/core/v1beta1/catalogsource_types.go +++ b/pkg/apis/core/v1beta1/catalogsource_types.go @@ -27,12 +27,19 @@ import ( "sigs.k8s.io/apiserver-runtime/pkg/builder/resource/resourcestrategy" ) +const ( + TypeReady = "Ready" + + ReasonContentsAvailable = "ContentsAvailable" +) + // +genclient // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object // CatalogSource // +k8s:openapi-gen=true // +kubebuilder:resource:scope=Cluster +// +kubebuilder:subresource:status type CatalogSource struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` @@ -110,7 +117,10 @@ func (in *CatalogSourceList) GetListMeta() *metav1.ListMeta { type CatalogSourceStatus struct { // The last time the image has been polled to ensure the image is up-to-date - LatestImagePoll *metav1.Time `json:"latestImagePoll"` + LatestImagePoll *metav1.Time `json:"latestImagePoll,omitempty"` + + // Conditions store the status conditions of the CatalogSource instances + Conditions []metav1.Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type" protobuf:"bytes,1,rep,name=conditions"` } func (in CatalogSourceStatus) SubResourceName() string { diff --git a/pkg/apis/core/v1beta1/zz_generated.deepcopy.go b/pkg/apis/core/v1beta1/zz_generated.deepcopy.go index 34ba5e87..25177a3a 100644 --- a/pkg/apis/core/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/core/v1beta1/zz_generated.deepcopy.go @@ -229,6 +229,13 @@ func (in *CatalogSourceStatus) DeepCopyInto(out *CatalogSourceStatus) { in, out := &in.LatestImagePoll, &out.LatestImagePoll *out = (*in).DeepCopy() } + if in.Conditions != nil { + in, out := &in.Conditions, &out.Conditions + *out = make([]v1.Condition, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CatalogSourceStatus. diff --git a/pkg/controllers/core/catalogsource_controller.go b/pkg/controllers/core/catalogsource_controller.go index f4e47c3e..78206dab 100644 --- a/pkg/controllers/core/catalogsource_controller.go +++ b/pkg/controllers/core/catalogsource_controller.go @@ -27,14 +27,17 @@ import ( batchv1 "k8s.io/api/batch/v1" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" ctrlutil "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/predicate" corev1beta1 "github.com/operator-framework/catalogd/pkg/apis/core/v1beta1" ) @@ -97,13 +100,30 @@ func (r *CatalogSourceReconciler) Reconcile(ctx context.Context, req ctrl.Reques if err := r.buildBundleMetadata(ctx, declCfg, catalogSource); err != nil { return ctrl.Result{}, err } + + // update CatalogSource status as "Ready" since at this point + // all catalog content should be available on cluster + updateStatusReady(&catalogSource) + if err := r.Client.Status().Update(ctx, &catalogSource); err != nil { + return ctrl.Result{}, fmt.Errorf("updating catalogsource status: %v", err) + } + return ctrl.Result{}, nil } +func updateStatusReady(catalogSource *corev1beta1.CatalogSource) { + meta.SetStatusCondition(&catalogSource.Status.Conditions, metav1.Condition{ + Type: corev1beta1.TypeReady, + Reason: corev1beta1.ReasonContentsAvailable, + Status: metav1.ConditionTrue, + Message: "catalog contents have been unpacked and are available on cluster", + }) +} + // SetupWithManager sets up the controller with the Manager. func (r *CatalogSourceReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). - For(&corev1beta1.CatalogSource{}). + For(&corev1beta1.CatalogSource{}, builder.WithPredicates(predicate.GenerationChangedPredicate{})). Complete(r) } From de8055b446dfcdf68351edaa2ab32e27676a84d7 Mon Sep 17 00:00:00 2001 From: Bryce Palmer Date: Fri, 31 Mar 2023 11:35:23 -0400 Subject: [PATCH 2/6] remove lastPollingTime status field Signed-off-by: Bryce Palmer --- pkg/apis/core/v1beta1/catalogsource_types.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/pkg/apis/core/v1beta1/catalogsource_types.go b/pkg/apis/core/v1beta1/catalogsource_types.go index 204efb31..953f69b2 100644 --- a/pkg/apis/core/v1beta1/catalogsource_types.go +++ b/pkg/apis/core/v1beta1/catalogsource_types.go @@ -115,10 +115,6 @@ func (in *CatalogSourceList) GetListMeta() *metav1.ListMeta { // CatalogSourceStatus defines the observed state of CatalogSource type CatalogSourceStatus struct { - - // The last time the image has been polled to ensure the image is up-to-date - LatestImagePoll *metav1.Time `json:"latestImagePoll,omitempty"` - // Conditions store the status conditions of the CatalogSource instances Conditions []metav1.Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type" protobuf:"bytes,1,rep,name=conditions"` } From 426ec3bd6db9fad87625c4e1070c4844762e27e7 Mon Sep 17 00:00:00 2001 From: Bryce Palmer Date: Fri, 31 Mar 2023 11:35:33 -0400 Subject: [PATCH 3/6] remove lastPollingTime status field Signed-off-by: Bryce Palmer --- pkg/apis/core/v1beta1/zz_generated.deepcopy.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/pkg/apis/core/v1beta1/zz_generated.deepcopy.go b/pkg/apis/core/v1beta1/zz_generated.deepcopy.go index 25177a3a..e20cd1b2 100644 --- a/pkg/apis/core/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/core/v1beta1/zz_generated.deepcopy.go @@ -225,10 +225,6 @@ func (in *CatalogSourceSpec) DeepCopy() *CatalogSourceSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *CatalogSourceStatus) DeepCopyInto(out *CatalogSourceStatus) { *out = *in - if in.LatestImagePoll != nil { - in, out := &in.LatestImagePoll, &out.LatestImagePoll - *out = (*in).DeepCopy() - } if in.Conditions != nil { in, out := &in.Conditions, &out.Conditions *out = make([]v1.Condition, len(*in)) From cccbd955773345a159de8b44a4e97fa9df4207bf Mon Sep 17 00:00:00 2001 From: Bryce Palmer Date: Fri, 31 Mar 2023 11:37:37 -0400 Subject: [PATCH 4/6] remove lastPollingTime status field Signed-off-by: Bryce Palmer --- config/crd/bases/core.catalogd.io_catalogsources.yaml | 5 ----- 1 file changed, 5 deletions(-) diff --git a/config/crd/bases/core.catalogd.io_catalogsources.yaml b/config/crd/bases/core.catalogd.io_catalogsources.yaml index f3310075..51858786 100644 --- a/config/crd/bases/core.catalogd.io_catalogsources.yaml +++ b/config/crd/bases/core.catalogd.io_catalogsources.yaml @@ -122,11 +122,6 @@ spec: - type type: object type: array - latestImagePoll: - description: The last time the image has been polled to ensure the - image is up-to-date - format: date-time - type: string type: object type: object served: true From bf10f4f778723fe25a6db228c7725346c09e913e Mon Sep 17 00:00:00 2001 From: Bryce Palmer Date: Fri, 7 Apr 2023 09:41:11 -0400 Subject: [PATCH 5/6] update status on errors Signed-off-by: Bryce Palmer --- pkg/apis/core/v1beta1/catalogsource_types.go | 1 + .../core/catalogsource_controller.go | 30 ++++++++++++++++++- 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/pkg/apis/core/v1beta1/catalogsource_types.go b/pkg/apis/core/v1beta1/catalogsource_types.go index 953f69b2..05fca25d 100644 --- a/pkg/apis/core/v1beta1/catalogsource_types.go +++ b/pkg/apis/core/v1beta1/catalogsource_types.go @@ -31,6 +31,7 @@ const ( TypeReady = "Ready" ReasonContentsAvailable = "ContentsAvailable" + ReasonUnpackError = "UnpackError" ) // +genclient diff --git a/pkg/controllers/core/catalogsource_controller.go b/pkg/controllers/core/catalogsource_controller.go index 78206dab..a710bb35 100644 --- a/pkg/controllers/core/catalogsource_controller.go +++ b/pkg/controllers/core/catalogsource_controller.go @@ -75,11 +75,19 @@ func (r *CatalogSourceReconciler) Reconcile(ctx context.Context, req ctrl.Reques if err != nil { if errors.IsNotFound(err) { if err = r.createUnpackJob(ctx, catalogSource); err != nil { + updateStatusError(&catalogSource, err) + if err := r.Client.Status().Update(ctx, &catalogSource); err != nil { + return ctrl.Result{}, fmt.Errorf("updating catalogsource status: %v", err) + } return ctrl.Result{}, err } // after creating the job requeue return ctrl.Result{Requeue: true}, nil } + updateStatusError(&catalogSource, err) + if err := r.Client.Status().Update(ctx, &catalogSource); err != nil { + return ctrl.Result{}, fmt.Errorf("updating catalogsource status: %v", err) + } return ctrl.Result{}, err } @@ -89,15 +97,27 @@ func (r *CatalogSourceReconciler) Reconcile(ctx context.Context, req ctrl.Reques if corev1beta1.IsUnpackPhaseError(err) { return ctrl.Result{RequeueAfter: 10 * time.Second}, nil } + updateStatusError(&catalogSource, err) + if err := r.Client.Status().Update(ctx, &catalogSource); err != nil { + return ctrl.Result{}, fmt.Errorf("updating catalogsource status: %v", err) + } return ctrl.Result{}, err } // TODO: Can we create these resources in parallel using goroutines? if err := r.buildPackages(ctx, declCfg, catalogSource); err != nil { + updateStatusError(&catalogSource, err) + if err := r.Client.Status().Update(ctx, &catalogSource); err != nil { + return ctrl.Result{}, fmt.Errorf("updating catalogsource status: %v", err) + } return ctrl.Result{}, err } if err := r.buildBundleMetadata(ctx, declCfg, catalogSource); err != nil { + updateStatusError(&catalogSource, err) + if err := r.Client.Status().Update(ctx, &catalogSource); err != nil { + return ctrl.Result{}, fmt.Errorf("updating catalogsource status: %v", err) + } return ctrl.Result{}, err } @@ -107,7 +127,6 @@ func (r *CatalogSourceReconciler) Reconcile(ctx context.Context, req ctrl.Reques if err := r.Client.Status().Update(ctx, &catalogSource); err != nil { return ctrl.Result{}, fmt.Errorf("updating catalogsource status: %v", err) } - return ctrl.Result{}, nil } @@ -120,6 +139,15 @@ func updateStatusReady(catalogSource *corev1beta1.CatalogSource) { }) } +func updateStatusError(catalogSource *corev1beta1.CatalogSource, err error) { + meta.SetStatusCondition(&catalogSource.Status.Conditions, metav1.Condition{ + Type: corev1beta1.TypeReady, + Status: metav1.ConditionFalse, + Reason: corev1beta1.ReasonUnpackError, + Message: err.Error(), + }) +} + // SetupWithManager sets up the controller with the Manager. func (r *CatalogSourceReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). From 80fbb30fd93c05e21e181a0de06cacfed3449377 Mon Sep 17 00:00:00 2001 From: Bryce Palmer Date: Fri, 7 Apr 2023 09:45:05 -0400 Subject: [PATCH 6/6] add TODO comment Signed-off-by: Bryce Palmer --- pkg/controllers/core/catalogsource_controller.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/pkg/controllers/core/catalogsource_controller.go b/pkg/controllers/core/catalogsource_controller.go index a710bb35..e99137e3 100644 --- a/pkg/controllers/core/catalogsource_controller.go +++ b/pkg/controllers/core/catalogsource_controller.go @@ -151,6 +151,13 @@ func updateStatusError(catalogSource *corev1beta1.CatalogSource, err error) { // SetupWithManager sets up the controller with the Manager. func (r *CatalogSourceReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). + // TODO: Due to us not having proper error handling, + // not having this results in the controller getting into + // an error state because once we update the status it requeues + // and then errors out when trying to create all the Packages again + // even though they already exist. This should be resolved by the fix + // for https://github.com/operator-framework/catalogd/issues/6. The fix for + // #6 should also remove the usage of `builder.WithPredicates(predicate.GenerationChangedPredicate{})` For(&corev1beta1.CatalogSource{}, builder.WithPredicates(predicate.GenerationChangedPredicate{})). Complete(r) }