Skip to content

Commit 5313780

Browse files
bigkevmcdbigkevmcd
committed
Simplify condition setup.
1 parent 0efad4b commit 5313780

File tree

3 files changed

+148
-119
lines changed

3 files changed

+148
-119
lines changed

‎api/v1alpha1/condition_types.go

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,6 @@ const (
1515
ClusterConnectedReason string = "ClusterConnected"
1616
// ClusterNotConnectedReason signals that a given cluster is not connected.
1717
ClusterNotConnectedReason string = "ClusterNotConnected"
18-
// SecretMissingAndNoConnectivityReason signals that a given secret is missing and there is no connectivity to the cluster.
19-
SecretMissingAndNoConnectivityReason string = "SecretMissingAndNoConnectivity"
2018

2119
// WaitingForCAPIClusterDeletionReason signals that this cluster has been
2220
// deleted, but the referenced CAPI Cluster still exists.
@@ -47,7 +45,4 @@ const (
4745
// ClusterProvisionedReason is the reason for the provisioned state being
4846
// set.
4947
ClusterProvisionedReason string = "ClusterProvisioned"
50-
51-
// ClusterConnectivity indicates if the cluster has connectivity
52-
ClusterConnectivity string = "ClusterConnectivity"
5348
)

‎controllers/gitopscluster_controller.go

Lines changed: 119 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -127,116 +127,76 @@ func (r *GitopsClusterReconciler) Reconcile(ctx context.Context, req ctrl.Reques
127127
}
128128

129129
// examine DeletionTimestamp to determine if object is under deletion
130-
if cluster.ObjectMeta.DeletionTimestamp.IsZero() {
131-
if cluster.Spec.SecretRef != nil || cluster.Spec.CAPIClusterRef != nil {
132-
if !controllerutil.ContainsFinalizer(cluster, GitOpsClusterFinalizer) {
133-
controllerutil.AddFinalizer(cluster, GitOpsClusterFinalizer)
134-
if err := r.Update(ctx, cluster); err != nil {
135-
return ctrl.Result{}, err
136-
}
137-
}
138-
}
139-
} else {
140-
if controllerutil.ContainsFinalizer(cluster, GitOpsClusterFinalizer) {
141-
err := r.reconcileDeletedReferences(ctx, cluster)
142-
if err != nil {
143-
return ctrl.Result{}, err
144-
}
145-
controllerutil.RemoveFinalizer(cluster, GitOpsClusterFinalizer)
146-
if err := r.Update(ctx, cluster); err != nil {
147-
return ctrl.Result{}, err
148-
}
149-
return ctrl.Result{}, nil
150-
}
130+
if !cluster.ObjectMeta.DeletionTimestamp.IsZero() {
131+
return r.finalize(ctx, cluster)
151132
}
152133

153-
if cluster.Spec.SecretRef != nil {
154-
name := types.NamespacedName{
155-
Namespace: cluster.GetNamespace(),
156-
Name: cluster.Spec.SecretRef.Name,
157-
}
134+
if cluster.Spec.CAPIClusterRef == nil && cluster.Spec.SecretRef == nil {
135+
return ctrl.Result{}, nil
136+
}
158137

159-
if metav1.HasAnnotation(cluster.ObjectMeta, GitOpsClusterProvisionedAnnotation) {
160-
conditions.MarkTrue(cluster, gitopsv1alpha1.ClusterProvisionedCondition, gitopsv1alpha1.ClusterProvisionedReason, "Cluster Provisioned annotation detected")
138+
if !controllerutil.ContainsFinalizer(cluster, GitOpsClusterFinalizer) {
139+
controllerutil.AddFinalizer(cluster, GitOpsClusterFinalizer)
140+
if err := r.Update(ctx, cluster); err != nil {
141+
return ctrl.Result{}, err
161142
}
162143

163-
var secret corev1.Secret
164-
if err := r.Get(ctx, name, &secret); err != nil {
165-
e := fmt.Errorf("failed to get secret %q: %w", name, err)
166-
if apierrors.IsNotFound(err) {
167-
// TODO: this could _possibly_ be controllable by the
168-
// `GitopsCluster` itself.
169-
log.Info("waiting for cluster secret to be available")
170-
conditions.MarkFalse(cluster, meta.ReadyCondition, gitopsv1alpha1.WaitingForSecretReason, e.Error())
171-
if err := r.Status().Update(ctx, cluster); err != nil {
172-
log.Error(err, "failed to update Cluster status")
173-
return ctrl.Result{}, err
174-
}
175-
return ctrl.Result{RequeueAfter: MissingSecretRequeueTime}, nil
176-
}
177-
conditions.MarkFalse(cluster, meta.ReadyCondition, gitopsv1alpha1.WaitingForSecretReason, e.Error())
178-
if err := r.Status().Update(ctx, cluster); err != nil {
179-
log.Error(err, "failed to update Cluster status")
180-
return ctrl.Result{}, err
181-
}
144+
return ctrl.Result{}, nil
145+
}
182146

183-
return ctrl.Result{}, e
147+
if cluster.Spec.CAPIClusterRef != nil {
148+
if err := r.reconcileCAPICluster(ctx, cluster); err != nil {
149+
return ctrl.Result{}, err
184150
}
185151

186-
log.Info("Secret found", "secret", name)
152+
return ctrl.Result{RequeueAfter: r.Options.DefaultRequeueTime}, nil
153+
}
187154

188-
conditions.MarkTrue(cluster, meta.ReadyCondition, gitopsv1alpha1.SecretFoundReason, "")
189-
if err := r.Status().Update(ctx, cluster); err != nil {
190-
log.Error(err, "failed to update Cluster status")
191-
return ctrl.Result{}, err
192-
}
155+
secretErr := r.checkClusterSecret(ctx, cluster)
156+
if secretErr == nil {
157+
log.Info("Secret found")
193158
}
194159

195-
if cluster.Spec.CAPIClusterRef != nil {
196-
name := types.NamespacedName{
197-
Namespace: cluster.GetNamespace(),
198-
Name: cluster.Spec.CAPIClusterRef.Name,
199-
}
200-
var capiCluster clusterv1.Cluster
201-
if err := r.Get(ctx, name, &capiCluster); err != nil {
202-
e := fmt.Errorf("failed to get CAPI cluster %q: %w", name, err)
203-
conditions.MarkFalse(cluster, meta.ReadyCondition, gitopsv1alpha1.WaitingForCAPIClusterReason, e.Error())
160+
var connectivityErr error
161+
// TODO: We should check for connectivity with CAPI clusters
162+
if secretErr == nil {
163+
connectivityErr = r.verifyConnectivity(ctx, cluster)
164+
}
165+
166+
if secretErr != nil {
167+
e := fmt.Errorf("failed to get referenced secret: %w", secretErr)
168+
if apierrors.IsNotFound(secretErr) {
169+
log.Info("waiting for cluster secret to be available")
170+
conditions.MarkFalse(cluster, meta.ReadyCondition, gitopsv1alpha1.WaitingForSecretReason, e.Error())
204171
if err := r.Status().Update(ctx, cluster); err != nil {
205172
log.Error(err, "failed to update Cluster status")
206173
return ctrl.Result{}, err
207174
}
208175

209-
return ctrl.Result{}, e
176+
return ctrl.Result{RequeueAfter: MissingSecretRequeueTime}, nil
210177
}
211178

212-
log.Info("CAPI Cluster found", "CAPI cluster", name)
213-
214-
if !capiCluster.Status.ControlPlaneReady {
215-
conditions.MarkFalse(cluster, meta.ReadyCondition, gitopsv1alpha1.WaitingForControlPlaneReadyStatusReason, "Waiting for ControlPlaneReady status")
216-
} else {
217-
conditions.MarkTrue(cluster, meta.ReadyCondition, gitopsv1alpha1.ControlPlaneReadyStatusReason, "")
218-
}
219-
if clusterv1.ClusterPhase(capiCluster.Status.Phase) == clusterv1.ClusterPhaseProvisioned {
220-
conditions.MarkTrue(cluster, gitopsv1alpha1.ClusterProvisionedCondition, gitopsv1alpha1.ClusterProvisionedReason, "CAPI Cluster has been provisioned")
221-
}
179+
conditions.MarkFalse(cluster, meta.ReadyCondition, gitopsv1alpha1.WaitingForSecretReason, e.Error())
222180
if err := r.Status().Update(ctx, cluster); err != nil {
223181
log.Error(err, "failed to update Cluster status")
224-
return ctrl.Result{}, err
182+
return ctrl.Result{}, e
225183
}
226-
}
227184

228-
if err := r.verifyConnectivity(ctx, cluster); err != nil {
229-
return ctrl.Result{}, err
185+
return ctrl.Result{}, e
230186
}
231187

232-
if conditions.IsTrue(cluster, gitopsv1alpha1.SecretFoundReason) {
233-
if conditions.IsTrue(cluster, gitopsv1alpha1.ClusterConnectivity) {
234-
conditions.MarkTrue(cluster, meta.ReadyCondition, gitopsv1alpha1.ClusterConnectedReason, "")
235-
} else {
236-
conditions.MarkFalse(cluster, meta.ReadyCondition, gitopsv1alpha1.ClusterNotConnectedReason, "No Connectivity")
188+
if connectivityErr != nil {
189+
e := fmt.Errorf("failed to connect to cluster with secret: %w", connectivityErr)
190+
conditions.MarkFalse(cluster, meta.ReadyCondition, gitopsv1alpha1.ClusterNotConnectedReason, e.Error())
191+
if err := r.Status().Update(ctx, cluster); err != nil {
192+
log.Error(err, "failed to update Cluster status")
193+
return ctrl.Result{}, e
237194
}
195+
196+
return ctrl.Result{}, e
238197
}
239198

199+
conditions.MarkTrue(cluster, meta.ReadyCondition, gitopsv1alpha1.ClusterConnectedReason, "cluster is connected")
240200
if err := r.Status().Update(ctx, cluster); err != nil {
241201
log.Error(err, "failed to update Cluster status")
242202
return ctrl.Result{}, err
@@ -385,12 +345,6 @@ func (r *GitopsClusterReconciler) requestsForCAPIClusterChange(ctx context.Conte
385345

386346
func (r *GitopsClusterReconciler) verifyConnectivity(ctx context.Context, cluster *gitopsv1alpha1.GitopsCluster) error {
387347
log := log.FromContext(ctx)
388-
389-
// avoid checking the cluster if it's under deletion.
390-
if !cluster.ObjectMeta.DeletionTimestamp.IsZero() {
391-
return nil
392-
}
393-
394348
log.Info("checking connectivity", "cluster", cluster.Name)
395349

396350
nsName := types.NamespacedName{Namespace: cluster.Namespace, Name: cluster.Name}
@@ -405,26 +359,80 @@ func (r *GitopsClusterReconciler) verifyConnectivity(ctx context.Context, cluste
405359

406360
config, err := r.restConfigFromSecret(ctx, cluster)
407361
if err != nil {
408-
conditions.MarkFalse(cluster, gitopsv1alpha1.ClusterConnectivity, gitopsv1alpha1.ClusterConnectionFailedReason, fmt.Sprintf("failed creating rest config from secret: %s", err))
362+
conditions.MarkFalse(cluster, meta.ReadyCondition, gitopsv1alpha1.ClusterConnectionFailedReason, fmt.Sprintf("failed creating rest config from secret: %s", err))
409363
if err := r.Status().Update(ctx, cluster); err != nil {
410364
log.Error(err, "failed to update Cluster status")
411365
return err
412366
}
413367

414-
return nil
368+
return err
415369
}
416370

417371
if _, err := client.New(config, client.Options{}); err != nil {
418-
conditions.MarkFalse(cluster, gitopsv1alpha1.ClusterConnectivity, gitopsv1alpha1.ClusterConnectionFailedReason, fmt.Sprintf("failed connecting to the cluster: %s", err))
372+
conditions.MarkFalse(cluster, meta.ReadyCondition, gitopsv1alpha1.ClusterConnectionFailedReason, fmt.Sprintf("failed connecting to the cluster: %s", err))
419373
if err := r.Status().Update(ctx, cluster); err != nil {
420374
log.Error(err, "failed to update Cluster status")
421375
return err
422376
}
423377

378+
return err
379+
}
380+
381+
conditions.MarkTrue(cluster, meta.ReadyCondition, gitopsv1alpha1.ClusterConnectionSucceededReason, "cluster connectivity is ok")
382+
if err := r.Status().Update(ctx, cluster); err != nil {
383+
log.Error(err, "failed to update Cluster status")
384+
return err
385+
}
386+
387+
return nil
388+
}
389+
390+
func (r *GitopsClusterReconciler) checkClusterSecret(ctx context.Context, cluster *gitopsv1alpha1.GitopsCluster) error {
391+
if cluster.Spec.SecretRef == nil {
424392
return nil
425393
}
394+
name := types.NamespacedName{
395+
Namespace: cluster.GetNamespace(),
396+
Name: cluster.Spec.SecretRef.Name,
397+
}
398+
399+
if metav1.HasAnnotation(cluster.ObjectMeta, GitOpsClusterProvisionedAnnotation) {
400+
conditions.MarkTrue(cluster, gitopsv1alpha1.ClusterProvisionedCondition, gitopsv1alpha1.ClusterProvisionedReason, "Cluster Provisioned annotation detected")
401+
}
402+
403+
var secret corev1.Secret
404+
// TODO This should check for a value key in the secret data.
405+
return r.Get(ctx, name, &secret)
406+
}
407+
408+
func (r *GitopsClusterReconciler) reconcileCAPICluster(ctx context.Context, cluster *gitopsv1alpha1.GitopsCluster) error {
409+
log := log.FromContext(ctx)
410+
name := types.NamespacedName{
411+
Namespace: cluster.GetNamespace(),
412+
Name: cluster.Spec.CAPIClusterRef.Name,
413+
}
414+
var capiCluster clusterv1.Cluster
415+
if err := r.Get(ctx, name, &capiCluster); err != nil {
416+
e := fmt.Errorf("failed to get CAPI cluster %q: %w", name, err)
417+
conditions.MarkFalse(cluster, meta.ReadyCondition, gitopsv1alpha1.WaitingForCAPIClusterReason, e.Error())
418+
if err := r.Status().Update(ctx, cluster); err != nil {
419+
log.Error(err, "failed to update Cluster status")
420+
return e
421+
}
422+
423+
return e
424+
}
425+
426+
log.Info("CAPI Cluster found", "CAPI cluster", name)
426427

427-
conditions.MarkTrue(cluster, gitopsv1alpha1.ClusterConnectivity, gitopsv1alpha1.ClusterConnectionSucceededReason, "cluster connectivity is ok")
428+
if !capiCluster.Status.ControlPlaneReady {
429+
conditions.MarkFalse(cluster, meta.ReadyCondition, gitopsv1alpha1.WaitingForControlPlaneReadyStatusReason, "Waiting for ControlPlaneReady status")
430+
} else {
431+
conditions.MarkTrue(cluster, meta.ReadyCondition, gitopsv1alpha1.ControlPlaneReadyStatusReason, "")
432+
}
433+
if clusterv1.ClusterPhase(capiCluster.Status.Phase) == clusterv1.ClusterPhaseProvisioned {
434+
conditions.MarkTrue(cluster, gitopsv1alpha1.ClusterProvisionedCondition, gitopsv1alpha1.ClusterProvisionedReason, "CAPI Cluster has been provisioned")
435+
}
428436
if err := r.Status().Update(ctx, cluster); err != nil {
429437
log.Error(err, "failed to update Cluster status")
430438
return err
@@ -433,6 +441,21 @@ func (r *GitopsClusterReconciler) verifyConnectivity(ctx context.Context, cluste
433441
return nil
434442
}
435443

444+
func (r *GitopsClusterReconciler) finalize(ctx context.Context, cluster *gitopsv1alpha1.GitopsCluster) (ctrl.Result, error) {
445+
if controllerutil.ContainsFinalizer(cluster, GitOpsClusterFinalizer) {
446+
err := r.reconcileDeletedReferences(ctx, cluster)
447+
if err != nil {
448+
return ctrl.Result{}, err
449+
}
450+
controllerutil.RemoveFinalizer(cluster, GitOpsClusterFinalizer)
451+
if err := r.Update(ctx, cluster); err != nil {
452+
return ctrl.Result{}, err
453+
}
454+
}
455+
456+
return ctrl.Result{}, nil
457+
}
458+
436459
func (r *GitopsClusterReconciler) restConfigFromSecret(ctx context.Context, cluster *gitopsv1alpha1.GitopsCluster) (*rest.Config, error) {
437460
log := log.FromContext(ctx)
438461

@@ -476,11 +499,11 @@ func (r *GitopsClusterReconciler) restConfigFromSecret(ctx context.Context, clus
476499
return nil, errors.New("no data present in cluster secret")
477500
}
478501

479-
restCfg, err := clientcmd.RESTConfigFromKubeConfig([]byte(data))
502+
restCfg, err := clientcmd.RESTConfigFromKubeConfig(data)
480503
if err != nil {
481-
log.Error(err, "unable to create kubconfig from GitOps Cluster secret data", "cluster", cluster.Name)
504+
log.Error(err, "unable to create KubeConfig from GitOps Cluster secret data", "cluster", cluster.Name)
482505

483-
return nil, err
506+
return nil, errors.New("failed to parse KubeConfig from Secret")
484507
}
485508

486509
return restCfg, nil

0 commit comments

Comments
 (0)