Skip to content

Commit f14a89e

Browse files
bigkevmcdbigkevmcd
committed
Improve the connectivity checking.
This makes the client check the server version which will be rejected if the credentials don't allow it.
1 parent 1d0c71d commit f14a89e

2 files changed

Lines changed: 52 additions & 20 deletions

File tree

‎api/v1alpha1/gitopscluster_types.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ func (in *GitopsCluster) SetConditions(conditions []metav1.Condition) {
6464
// +kubebuilder:printcolumn:name="ClusterConnectivity",type="string",JSONPath=".status.conditions[?(@.type==\"ClusterConnectivity\")].status",description=""
6565

6666
// GitopsCluster is the Schema for the gitopsclusters API
67-
// +kubebuilder:validation:XValidation:rule="has(self.spec)",message="must confgure spec"
67+
// +kubebuilder:validation:XValidation:rule="has(self.spec)",message="must configure spec"
6868
type GitopsCluster struct {
6969
metav1.TypeMeta `json:",inline"`
7070
metav1.ObjectMeta `json:"metadata,omitempty"`

‎controllers/gitopscluster_controller.go‎

Lines changed: 51 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -30,14 +30,17 @@ import (
3030
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3131
"k8s.io/apimachinery/pkg/runtime"
3232
"k8s.io/apimachinery/pkg/types"
33+
"k8s.io/client-go/discovery"
3334
"k8s.io/client-go/rest"
3435
"k8s.io/client-go/tools/clientcmd"
3536
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
3637
ctrl "sigs.k8s.io/controller-runtime"
38+
"sigs.k8s.io/controller-runtime/pkg/builder"
3739
"sigs.k8s.io/controller-runtime/pkg/client"
3840
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
3941
"sigs.k8s.io/controller-runtime/pkg/handler"
4042
"sigs.k8s.io/controller-runtime/pkg/log"
43+
"sigs.k8s.io/controller-runtime/pkg/predicate"
4144

4245
gitopsv1alpha1 "github.com/weaveworks/cluster-controller/api/v1alpha1"
4346
)
@@ -119,7 +122,7 @@ func (r *GitopsClusterReconciler) Reconcile(ctx context.Context, req ctrl.Reques
119122

120123
conditions.MarkFalse(cluster, meta.ReadyCondition, gitopsv1alpha1.CAPINotEnabled, e.Error())
121124

122-
if err := r.Status().Update(ctx, cluster); err != nil {
125+
if err := r.patchStatus(ctx, req.NamespacedName, cluster.Status); err != nil {
123126
log.Error(err, "failed to update Cluster status")
124127
return ctrl.Result{}, err
125128
}
@@ -168,16 +171,16 @@ func (r *GitopsClusterReconciler) Reconcile(ctx context.Context, req ctrl.Reques
168171
if apierrors.IsNotFound(err) {
169172
// TODO: this could _possibly_ be controllable by the
170173
// `GitopsCluster` itself.
171-
log.Info("waiting for cluster secret to be available")
174+
log.Info("Waiting for cluster secret to be available")
172175
conditions.MarkFalse(cluster, meta.ReadyCondition, gitopsv1alpha1.WaitingForSecretReason, e.Error())
173-
if err := r.Status().Update(ctx, cluster); err != nil {
176+
if err := r.patchStatus(ctx, req.NamespacedName, cluster.Status); err != nil {
174177
log.Error(err, "failed to update Cluster status")
175178
return ctrl.Result{}, err
176179
}
177180
return ctrl.Result{RequeueAfter: MissingSecretRequeueTime}, nil
178181
}
179182
conditions.MarkFalse(cluster, meta.ReadyCondition, gitopsv1alpha1.WaitingForSecretReason, e.Error())
180-
if err := r.Status().Update(ctx, cluster); err != nil {
183+
if err := r.patchStatus(ctx, req.NamespacedName, cluster.Status); err != nil {
181184
log.Error(err, "failed to update Cluster status")
182185
return ctrl.Result{}, err
183186
}
@@ -188,7 +191,7 @@ func (r *GitopsClusterReconciler) Reconcile(ctx context.Context, req ctrl.Reques
188191
log.Info("Secret found", "secret", name)
189192

190193
conditions.MarkTrue(cluster, meta.ReadyCondition, gitopsv1alpha1.SecretFoundReason, "Referenced secret is available")
191-
if err := r.Status().Update(ctx, cluster); err != nil {
194+
if err := r.patchStatus(ctx, req.NamespacedName, cluster.Status); err != nil {
192195
log.Error(err, "failed to update Cluster status")
193196
return ctrl.Result{}, err
194197
}
@@ -203,7 +206,7 @@ func (r *GitopsClusterReconciler) Reconcile(ctx context.Context, req ctrl.Reques
203206
if err := r.Get(ctx, name, &capiCluster); err != nil {
204207
e := fmt.Errorf("failed to get CAPI cluster %q: %w", name, err)
205208
conditions.MarkFalse(cluster, meta.ReadyCondition, gitopsv1alpha1.WaitingForCAPIClusterReason, e.Error())
206-
if err := r.Status().Update(ctx, cluster); err != nil {
209+
if err := r.patchStatus(ctx, req.NamespacedName, cluster.Status); err != nil {
207210
log.Error(err, "failed to update Cluster status")
208211
return ctrl.Result{}, err
209212
}
@@ -221,7 +224,7 @@ func (r *GitopsClusterReconciler) Reconcile(ctx context.Context, req ctrl.Reques
221224
if clusterv1.ClusterPhase(capiCluster.Status.Phase) == clusterv1.ClusterPhaseProvisioned {
222225
conditions.MarkTrue(cluster, gitopsv1alpha1.ClusterProvisionedCondition, gitopsv1alpha1.ClusterProvisionedReason, "CAPI Cluster has been provisioned")
223226
}
224-
if err := r.Status().Update(ctx, cluster); err != nil {
227+
if err := r.patchStatus(ctx, req.NamespacedName, cluster.Status); err != nil {
225228
log.Error(err, "failed to update Cluster status")
226229
return ctrl.Result{}, err
227230
}
@@ -250,7 +253,7 @@ func (r *GitopsClusterReconciler) reconcileDeletedReferences(ctx context.Context
250253
conditions.MarkFalse(gc, meta.ReadyCondition,
251254
gitopsv1alpha1.WaitingForCAPIClusterDeletionReason,
252255
"waiting for CAPI cluster to be deleted")
253-
if err := r.Status().Update(ctx, gc); err != nil {
256+
if err := r.patchStatus(ctx, client.ObjectKeyFromObject(gc), gc.Status); err != nil {
254257
log.Error(err, "failed to update Cluster status")
255258
return err
256259
}
@@ -270,7 +273,7 @@ func (r *GitopsClusterReconciler) reconcileDeletedReferences(ctx context.Context
270273
conditions.MarkFalse(gc, meta.ReadyCondition,
271274
gitopsv1alpha1.WaitingForSecretDeletionReason,
272275
"waiting for access secret to be deleted")
273-
if err := r.Status().Update(ctx, gc); err != nil {
276+
if err := r.patchStatus(ctx, client.ObjectKeyFromObject(gc), gc.Status); err != nil {
274277
log.Error(err, "failed to update Cluster status")
275278
return err
276279
}
@@ -291,7 +294,7 @@ func (r *GitopsClusterReconciler) SetupWithManager(mgr ctrl.Manager) error {
291294
}
292295

293296
builder := ctrl.NewControllerManagedBy(mgr).
294-
For(&gitopsv1alpha1.GitopsCluster{}).
297+
For(&gitopsv1alpha1.GitopsCluster{}, builder.WithPredicates(predicate.GenerationChangedPredicate{})).
295298
Watches(
296299
&corev1.Secret{},
297300
handler.EnqueueRequestsFromMapFunc(r.requestsForSecretChange),
@@ -380,41 +383,58 @@ func (r *GitopsClusterReconciler) verifyConnectivity(ctx context.Context, cluste
380383
return nil
381384
}
382385

383-
log.Info("checking connectivity", "cluster", cluster.Name)
386+
log.Info("Checking connectivity", "cluster", cluster.Name)
384387

385388
nsName := types.NamespacedName{Namespace: cluster.Namespace, Name: cluster.Name}
386389

387-
lastCheck := r.lastConnectivityCheck[nsName.String()]
390+
// lastCheck := r.lastConnectivityCheck[nsName.String()]
388391

389-
if time.Since(lastCheck) < 30*time.Second {
390-
return nil
391-
}
392+
// if time.Since(lastCheck) < 30*time.Second {
393+
// return nil
394+
// }
392395

393396
r.lastConnectivityCheck[nsName.String()] = time.Now()
394397

395398
config, err := r.restConfigFromSecret(ctx, cluster)
396399
if err != nil {
400+
log.Error(err, "failed to parse the config from the secret")
397401
conditions.MarkFalse(cluster, gitopsv1alpha1.ClusterConnectivity, gitopsv1alpha1.ClusterConnectionFailedReason, fmt.Sprintf("failed creating rest config from secret: %s", err))
398-
if err := r.Status().Update(ctx, cluster); err != nil {
402+
if err := r.patchStatus(ctx, client.ObjectKeyFromObject(cluster), cluster.Status); err != nil {
399403
log.Error(err, "failed to update Cluster status")
400404
return err
401405
}
402406

403407
return nil
404408
}
405409

406-
if _, err := client.New(config, client.Options{}); err != nil {
410+
k8sClient, err := discovery.NewDiscoveryClientForConfig(config)
411+
if err != nil {
412+
log.Error(err, "failed to create a discovery client with the config")
407413
conditions.MarkFalse(cluster, gitopsv1alpha1.ClusterConnectivity, gitopsv1alpha1.ClusterConnectionFailedReason, fmt.Sprintf("failed connecting to the cluster: %s", err))
408-
if err := r.Status().Update(ctx, cluster); err != nil {
414+
if err := r.patchStatus(ctx, client.ObjectKeyFromObject(cluster), cluster.Status); err != nil {
415+
log.Error(err, "failed to update Cluster status")
416+
return err
417+
}
418+
419+
return nil
420+
}
421+
422+
version, err := k8sClient.ServerVersion()
423+
if err != nil {
424+
log.Error(err, "failed to get the server version")
425+
conditions.MarkFalse(cluster, gitopsv1alpha1.ClusterConnectivity, gitopsv1alpha1.ClusterConnectionFailedReason, fmt.Sprintf("failed to get server version: %s", err))
426+
if err := r.patchStatus(ctx, client.ObjectKeyFromObject(cluster), cluster.Status); err != nil {
409427
log.Error(err, "failed to update Cluster status")
410428
return err
411429
}
412430

413431
return nil
414432
}
415433

434+
log.Info("Server version", "version", version.String())
435+
416436
conditions.MarkTrue(cluster, gitopsv1alpha1.ClusterConnectivity, gitopsv1alpha1.ClusterConnectionSucceededReason, "cluster connectivity is ok")
417-
if err := r.Status().Update(ctx, cluster); err != nil {
437+
if err := r.patchStatus(ctx, client.ObjectKeyFromObject(cluster), cluster.Status); err != nil {
418438
log.Error(err, "failed to update Cluster status")
419439
return err
420440
}
@@ -474,3 +494,15 @@ func (r *GitopsClusterReconciler) restConfigFromSecret(ctx context.Context, clus
474494

475495
return restCfg, nil
476496
}
497+
498+
func (r *GitopsClusterReconciler) patchStatus(ctx context.Context, key client.ObjectKey, newStatus gitopsv1alpha1.GitopsClusterStatus) error {
499+
var cluster gitopsv1alpha1.GitopsCluster
500+
if err := r.Get(ctx, key, &cluster); err != nil {
501+
return err
502+
}
503+
504+
patch := client.MergeFrom(cluster.DeepCopy())
505+
cluster.Status = newStatus
506+
507+
return r.Status().Patch(ctx, &cluster, patch)
508+
}

0 commit comments

Comments
 (0)