Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 KCP should set a timeout when trying to connect to remote cluster #2708

Merged
merged 1 commit into from
Mar 18, 2020
Merged

🐛 KCP should set a timeout when trying to connect to remote cluster #2708

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions controlplane/kubeadm/controllers/kubeadm_control_plane_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ func (r *KubeadmControlPlaneReconciler) Reconcile(req ctrl.Request) (res ctrl.Re
reterr = nil
}
}

// Always attempt to update status.
if err := r.updateStatus(ctx, kcp, cluster); err != nil {
logger.Error(err, "Failed to update KubeadmControlPlane Status")
Expand All @@ -181,6 +182,7 @@ func (r *KubeadmControlPlaneReconciler) Reconcile(req ctrl.Request) (res ctrl.Re
logger.Error(err, "Failed to patch KubeadmControlPlane")
reterr = kerrors.NewAggregate([]error{reterr, err})
}

}()

if !kcp.ObjectMeta.DeletionTimestamp.IsZero() {
Expand Down Expand Up @@ -323,6 +325,11 @@ func (r *KubeadmControlPlaneReconciler) updateStatus(ctx context.Context, kcp *c
kcp.Status.ReadyReplicas = 0
kcp.Status.UnavailableReplicas = replicas

// Return early if the deletion timestamp is set, we don't want to try to connect to the workload cluster.
if !kcp.DeletionTimestamp.IsZero() {
return nil
}

workloadCluster, err := r.managementCluster.GetWorkloadCluster(ctx, util.ObjectKey(cluster))
if err != nil {
return errors.Wrap(err, "failed to create remote cluster client")
Expand Down
8 changes: 5 additions & 3 deletions controlplane/kubeadm/internal/cluster.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"crypto/tls"
"crypto/x509"
"fmt"
"time"

"github.com/pkg/errors"
corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -66,18 +67,19 @@ func (m *Management) GetMachinesForCluster(ctx context.Context, cluster client.O
// GetWorkloadCluster builds a cluster object.
// The cluster comes with an etcd client generator to connect to any etcd pod living on a managed machine.
func (m *Management) GetWorkloadCluster(ctx context.Context, clusterKey client.ObjectKey) (WorkloadCluster, error) {
// TODO(chuckha): Unroll remote.NewClusterClient if we are unhappy with getting a restConfig twice.
// TODO(chuckha): Inject this dependency.
// TODO(chuckha): memoize this function. The workload client only exists as long as a reconciliation loop.
restConfig, err := remote.RESTConfig(ctx, m.Client, clusterKey)
restConfig.Timeout = 30 * time.Second
vincepri marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return nil, err
}

c, err := remote.NewClusterClient(ctx, m.Client, clusterKey, scheme.Scheme)
c, err := client.New(restConfig, client.Options{Scheme: scheme.Scheme})
if err != nil {
return nil, err
return nil, errors.Wrapf(err, "failed to create client for workload cluster %v", clusterKey)
}

etcdCASecret := &corev1.Secret{}
etcdCAObjectKey := ctrlclient.ObjectKey{
Namespace: clusterKey.Namespace,
Expand Down