diff --git a/api/v1alpha1/condition_types.go b/api/v1alpha1/condition_types.go new file mode 100644 index 0000000..0442dba --- /dev/null +++ b/api/v1alpha1/condition_types.go @@ -0,0 +1,20 @@ +// Copyright 2022. +// SPDX-FileCopyrightText: 2022 SAP SE or an SAP affiliate company and Open Component Model contributors. +// +// SPDX-License-Identifier: Apache-2.0 + +package v1alpha1 + +const ( + // PatchFailedReason is used when we couldn't patch an object. + PatchFailedReason = "PatchFailed" + + // SnapshotGetFailedReason is used when the needed snapshot does not exist. + SnapshotGetFailedReason = "SnapshotGetFailed" + + // AuthenticateGetFailedReason is used when the needed authentication does not exist. + CredentialsNotFoundReason = "CredentialsNotFound" + + // GitRepositoryPushFailedReason is used when the needed pushing to a git repository failed. + GitRepositoryPushFailedReason = "GitRepositoryPushFailed" +) diff --git a/api/v1alpha1/gitsync_types.go b/api/v1alpha1/gitsync_types.go index e702ff5..a31897f 100644 --- a/api/v1alpha1/gitsync_types.go +++ b/api/v1alpha1/gitsync_types.go @@ -6,6 +6,8 @@ package v1alpha1 import ( + "time" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -38,6 +40,30 @@ type GitSyncSpec struct { // GitSyncStatus defines the observed state of GitSync type GitSyncStatus struct { Digest string `json:"digest,omitempty"` + + // ObservedGeneration is the last reconciled generation. + // +optional + ObservedGeneration int64 `json:"observedGeneration,omitempty"` + // +optional + // +kubebuilder:printcolumn:name="Ready",type="string",JSONPath=".status.conditions[?(@.type==\"Ready\")].status",description="" + // +kubebuilder:printcolumn:name="Status",type="string",JSONPath=".status.conditions[?(@.type==\"Ready\")].message",description="" + Conditions []metav1.Condition `json:"conditions,omitempty"` +} + +// GetConditions returns the conditions of the ComponentVersion. +func (in *GitSync) GetConditions() []metav1.Condition { + return in.Status.Conditions +} + +// SetConditions sets the conditions of the ComponentVersion. +func (in *GitSync) SetConditions(conditions []metav1.Condition) { + in.Status.Conditions = conditions +} + +// GetRequeueAfter returns the duration after which the ComponentVersion must be +// reconciled again. +func (in GitSync) GetRequeueAfter() time.Duration { + return in.Spec.Interval.Duration } //+kubebuilder:object:root=true diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index d370f75..e1392c6 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -22,6 +22,7 @@ limitations under the License. package v1alpha1 import ( + "k8s.io/apimachinery/pkg/apis/meta/v1" runtime "k8s.io/apimachinery/pkg/runtime" ) @@ -46,7 +47,7 @@ func (in *GitSync) DeepCopyInto(out *GitSync) { 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 GitSync. @@ -126,6 +127,13 @@ func (in *GitSyncSpec) DeepCopy() *GitSyncSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *GitSyncStatus) DeepCopyInto(out *GitSyncStatus) { *out = *in + 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 GitSyncStatus. diff --git a/config/crd/bases/delivery.ocm.software_gitsyncs.yaml b/config/crd/bases/delivery.ocm.software_gitsyncs.yaml index f2fb798..15fc26a 100644 --- a/config/crd/bases/delivery.ocm.software_gitsyncs.yaml +++ b/config/crd/bases/delivery.ocm.software_gitsyncs.yaml @@ -106,8 +106,80 @@ spec: status: description: GitSyncStatus defines the observed state of GitSync properties: + conditions: + 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 digest: type: string + observedGeneration: + description: ObservedGeneration is the last reconciled generation. + format: int64 + type: integer type: object type: object served: true diff --git a/config/manager/deployment.yaml b/config/manager/deployment.yaml index dab2cce..9ee2bfc 100644 --- a/config/manager/deployment.yaml +++ b/config/manager/deployment.yaml @@ -2,7 +2,7 @@ apiVersion: v1 kind: Namespace metadata: labels: - control-plane: git-sync-controller + app: git-sync-controller name: ocm-system --- apiVersion: apps/v1 @@ -11,18 +11,18 @@ metadata: name: git-sync-controller namespace: ocm-system labels: - control-plane: git-sync-controller + app: git-sync-controller spec: selector: matchLabels: - control-plane: git-sync-controller + app: git-sync-controller replicas: 1 template: metadata: annotations: kubectl.kubernetes.io/default-container: manager labels: - control-plane: git-sync-controller + app: git-sync-controller spec: securityContext: runAsNonRoot: true diff --git a/controllers/gitsync_controller.go b/controllers/gitsync_controller.go index b2c65a5..fe053e5 100644 --- a/controllers/gitsync_controller.go +++ b/controllers/gitsync_controller.go @@ -7,12 +7,15 @@ package controllers import ( "context" + "errors" "fmt" - "time" + "github.com/fluxcd/pkg/apis/meta" + "github.com/fluxcd/pkg/runtime/conditions" "github.com/fluxcd/pkg/runtime/patch" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" @@ -44,71 +47,139 @@ type GitSyncReconciler struct { // Reconcile is part of the main kubernetes reconciliation loop which aims to // move the current state of the cluster closer to the desired state. func (r *GitSyncReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + var ( + result ctrl.Result + retErr error + ) + log := log.FromContext(ctx) log.V(4).Info("starting reconcile loop for snapshot") - gitSync := &v1alpha1.GitSync{} - if err := r.Get(ctx, req.NamespacedName, gitSync); err != nil { + obj := &v1alpha1.GitSync{} + if err := r.Get(ctx, req.NamespacedName, obj); err != nil { if apierrors.IsNotFound(err) { return ctrl.Result{}, nil } return ctrl.Result{}, fmt.Errorf("failed to get git sync object: %w", err) } - log.V(4).Info("found reconciling object", "gitSync", gitSync) + log.V(4).Info("found reconciling object", "gitSync", obj) + + // The replication controller doesn't need a shouldReconcile, because it should always reconcile, + // that is its purpose. + patchHelper, err := patch.NewHelper(obj, r.Client) + if err != nil { + retErr = errors.Join(retErr, err) + conditions.MarkFalse(obj, meta.ReadyCondition, v1alpha1.PatchFailedReason, err.Error()) + + return ctrl.Result{}, retErr + } + + // Always attempt to patch the object and status after each reconciliation. + defer func() { + // Patching has not been set up, or the controller errored earlier. + if patchHelper == nil { + return + } - if gitSync.Status.Digest != "" { - log.Info("GitSync object already synced; status contains digest information", "digest", gitSync.Status.Digest) + if condition := conditions.Get(obj, meta.StalledCondition); condition != nil && condition.Status == metav1.ConditionTrue { + conditions.Delete(obj, meta.ReconcilingCondition) + } + + // Check if it's a successful reconciliation. + // We don't set Requeue in case of error, so we can safely check for Requeue. + if result.RequeueAfter == obj.GetRequeueAfter() && !result.Requeue && retErr == nil { + // Remove the reconciling condition if it's set. + conditions.Delete(obj, meta.ReconcilingCondition) + + // Set the return err as the ready failure message if the resource is not ready, but also not reconciling or stalled. + if ready := conditions.Get(obj, meta.ReadyCondition); ready != nil && ready.Status == metav1.ConditionFalse && !conditions.IsStalled(obj) { + retErr = errors.New(conditions.GetMessage(obj, meta.ReadyCondition)) + } + } + + // If still reconciling then reconciliation did not succeed, set to ProgressingWithRetry to + // indicate that reconciliation will be retried. + if conditions.IsReconciling(obj) { + reconciling := conditions.Get(obj, meta.ReconcilingCondition) + reconciling.Reason = meta.ProgressingWithRetryReason + conditions.Set(obj, reconciling) + } + + // If not reconciling or stalled than mark Ready=True + if !conditions.IsReconciling(obj) && + !conditions.IsStalled(obj) && + retErr == nil { + conditions.MarkTrue(obj, meta.ReadyCondition, meta.SucceededReason, "Reconciliation success") + } + + // Set status observed generation option if the component is stalled or ready. + if conditions.IsStalled(obj) || conditions.IsReady(obj) { + obj.Status.ObservedGeneration = obj.Generation + } + + // Update the object. + if err := patchHelper.Patch(ctx, obj); err != nil { + retErr = errors.Join(retErr, err) + } + }() + + // it's important that this happens here so any residual status condition can be overwritten / set. + if obj.Status.Digest != "" { + log.Info("GitSync object already synced; status contains digest information", "digest", obj.Status.Digest) return ctrl.Result{}, nil } snapshot := &ocmv1.Snapshot{} if err := r.Get(ctx, types.NamespacedName{ - Namespace: gitSync.Spec.SnapshotRef.Namespace, - Name: gitSync.Spec.SnapshotRef.Name, + Namespace: obj.Spec.SnapshotRef.Namespace, + Name: obj.Spec.SnapshotRef.Name, }, snapshot); err != nil { - return ctrl.Result{}, fmt.Errorf("failed to find snapshot: %w", err) + retErr = fmt.Errorf("failed to find snapshot: %w", err) + conditions.MarkFalse(obj, meta.ReadyCondition, v1alpha1.SnapshotGetFailedReason, retErr.Error()) + + return ctrl.Result{}, retErr } + authSecret := &corev1.Secret{} if err := r.Get(ctx, types.NamespacedName{ - Namespace: gitSync.Spec.AuthRef.Namespace, - Name: gitSync.Spec.AuthRef.Name, + Namespace: obj.Spec.AuthRef.Namespace, + Name: obj.Spec.AuthRef.Name, }, authSecret); err != nil { - return ctrl.Result{}, fmt.Errorf("failed to find authentication secret: %w", err) + retErr = fmt.Errorf("failed to find authentication secret: %w", err) + conditions.MarkFalse(obj, meta.ReadyCondition, v1alpha1.CredentialsNotFoundReason, retErr.Error()) + + return ctrl.Result{}, retErr } // trim any trailing `/` and then just add. log.V(4).Info("crafting artifact URL to download from", "url", snapshot.Status.RepositoryURL) opts := &providers.PushOptions{ - URL: gitSync.Spec.URL, - Message: gitSync.Spec.CommitTemplate.Message, - Name: gitSync.Spec.CommitTemplate.Name, - Email: gitSync.Spec.CommitTemplate.Email, + URL: obj.Spec.URL, + Message: obj.Spec.CommitTemplate.Message, + Name: obj.Spec.CommitTemplate.Name, + Email: obj.Spec.CommitTemplate.Email, Snapshot: snapshot, - Branch: gitSync.Spec.Branch, - SubPath: gitSync.Spec.SubPath, + Branch: obj.Spec.Branch, + SubPath: obj.Spec.SubPath, } + r.parseAuthSecret(authSecret, opts) digest, err := r.Git.Push(ctx, opts) if err != nil { - return ctrl.Result{}, fmt.Errorf("failed to push to git repository: %w", err) - } - // Initialize the patch helper. - patchHelper, err := patch.NewHelper(gitSync, r.Client) - if err != nil { - return ctrl.Result{ - RequeueAfter: 1 * time.Minute, - }, fmt.Errorf("failed to create patch helper: %w", err) - } + retErr = fmt.Errorf("failed to push to git repository: %w", err) + conditions.MarkFalse(obj, meta.ReadyCondition, v1alpha1.GitRepositoryPushFailedReason, retErr.Error()) - gitSync.Status.Digest = digest - if err := patchHelper.Patch(ctx, gitSync); err != nil { - return ctrl.Result{ - RequeueAfter: 1 * time.Minute, - }, fmt.Errorf("failed to patch git sync object: %w", err) + return ctrl.Result{}, retErr } - log.V(4).Info("patch successful") - return ctrl.Result{}, nil + obj.Status.Digest = digest + + // Remove any stale Ready condition, most likely False, set above. Its value + // is derived from the overall result of the reconciliation in the deferred + // block at the very end. + conditions.Delete(obj, meta.ReadyCondition) + + return result, retErr } // SetupWithManager sets up the controller with the Manager. diff --git a/controllers/gitsync_controller_test.go b/controllers/gitsync_controller_test.go index b013ebc..bfd6bfb 100644 --- a/controllers/gitsync_controller_test.go +++ b/controllers/gitsync_controller_test.go @@ -4,6 +4,8 @@ import ( "context" "testing" + "github.com/fluxcd/pkg/apis/meta" + "github.com/fluxcd/pkg/runtime/conditions" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" v1 "k8s.io/api/core/v1" @@ -85,6 +87,7 @@ func TestGitSyncReconciler(t *testing.T) { require.NoError(t, err) assert.Equal(t, "test-digest", gitSync.Status.Digest) + assert.True(t, conditions.IsTrue(gitSync, meta.ReadyCondition)) } type mockGit struct { diff --git a/go.mod b/go.mod index f30c02c..3ed5179 100644 --- a/go.mod +++ b/go.mod @@ -4,6 +4,7 @@ go 1.20 require ( github.com/Masterminds/semver/v3 v3.2.0 + github.com/fluxcd/pkg/apis/meta v0.19.0 github.com/fluxcd/pkg/runtime v0.27.0 github.com/fluxcd/pkg/tar v0.2.0 github.com/go-git/go-git/v5 v5.6.0 @@ -65,7 +66,6 @@ require ( github.com/emirpasic/gods v1.18.1 // indirect github.com/evanphx/json-patch v5.6.0+incompatible // indirect github.com/evanphx/json-patch/v5 v5.6.0 // indirect - github.com/fluxcd/pkg/apis/meta v0.19.0 // indirect github.com/fsnotify/fsnotify v1.6.0 // indirect github.com/fvbommel/sortorder v1.0.2 // indirect github.com/ghodss/yaml v1.0.0 // indirect diff --git a/pkg/providers/gogit/git.go b/pkg/providers/gogit/git.go index b693114..1fdf84a 100644 --- a/pkg/providers/gogit/git.go +++ b/pkg/providers/gogit/git.go @@ -5,7 +5,9 @@ package gogit import ( + "compress/gzip" "context" + "errors" "fmt" "os" "path/filepath" @@ -95,7 +97,9 @@ func (g *Git) Push(ctx context.Context, opts *pkg.PushOptions) (string, error) { return "", fmt.Errorf("failed to fetch blob for digest: %w", err) } - if err = tar.Untar(blob, dir, tar.WithMaxUntarSize(-1)); err != nil { + // we only care about the error if it is NOT a header error. Otherwise, we assume the content + // wasn't compressed. + if err = tar.Untar(blob, dir, tar.WithMaxUntarSize(-1)); err != nil && !errors.Is(err, gzip.ErrHeader) { return "", fmt.Errorf("failed to untar first layer: %w", err) }