OCPEDGE-2037: Improve etcd handling

Author: pacevedom

pkg/controllers/etcd.go

@@ -169,30 +169,59 @@ func stopMicroshiftEtcdScopeIfExists() error {
 }
 
 func checkIfEtcdIsReady(ctx context.Context) error {
-	client, err := getEtcdClient(ctx)
-	if err != nil {
-		return fmt.Errorf("failed to obtain etcd client: %v", err)
-	}
-	defer func() { _ = client.Close() }()
+	var client *clientv3.Client
+	defer func() {
+		if client != nil {
+			_ = client.Close()
+		}
+	}()
 
-	s, err := client.Status(ctx, "localhost:2379")
-	if err != nil {
-		return fmt.Errorf("failed to get etcd status: %v", err)
-	}
-	if s.IsLearner {
-		return nil
-	}
+	for attempt := 0; attempt < HealthCheckRetries; attempt++ {
+		if client == nil {
+			var err error
+			client, err = getEtcdClient(ctx)
+			if err != nil {
+				klog.Infof("failed to obtain etcd client: %v", err)
+				if attempt < HealthCheckRetries-1 {
+					time.Sleep(HealthCheckWait)
+					continue
+				}
+				return fmt.Errorf("failed to obtain etcd client after %d attempts: %v", HealthCheckRetries, err)
+			}
+		}
+
+		s, err := client.Status(ctx, "localhost:2379")
+		if err != nil {
+			_ = client.Close()
+			client = nil
+			klog.Infof("failed to get etcd status: %v", err)
+			if attempt < HealthCheckRetries-1 {
+				time.Sleep(HealthCheckWait)
+				continue
+			}
+			return fmt.Errorf("failed to get etcd status after %d attempts: %v", HealthCheckRetries, err)
+		}
+
+		// If my own instance is a learner I dont need to check readiness because apiserver is going
+		// to connect to other voting members in the cluster.
+		if s.IsLearner {
+			return nil
+		}
 
-	for i := 0; i < HealthCheckRetries; i++ {
-		time.Sleep(HealthCheckWait)
 		if _, err = client.Get(ctx, "health"); err == nil {
 			return nil
 		} else {
+			_ = client.Close()
+			client = nil
 			klog.Infof("etcd not ready yet: %v", err)
 			if err == context.Canceled {
 				return err
 			}
 		}
+
+		if attempt < HealthCheckRetries-1 {
+			time.Sleep(HealthCheckWait)
+		}
 	}
 	return fmt.Errorf("etcd still not healthy after checking %d times", HealthCheckRetries)
 }

pkg/controllers/kube-apiserver.go

@@ -164,7 +164,7 @@ func (s *KubeAPIServer) configure(ctx context.Context, cfg *config.Config) error
 		}
 	}
 
-	etcdServers, err := discoverEtcdServers(ctx, s.configuration.Node.HostnameOverride, s.configuration.BootstrapKubeConfigPath())
+	etcdServers, err := discoverEtcdServers(ctx, s.configuration.BootstrapKubeConfigPath())
 	if err != nil {
 		return fmt.Errorf("failed to discover etcd servers: %w", err)
 	}
@@ -412,7 +412,7 @@ func (s *KubeAPIServer) Run(ctx context.Context, ready chan<- struct{}, stopped
 	}
 }
 
-func discoverEtcdServers(ctx context.Context, hostname, kubeconfigPath string) ([]string, error) {
+func discoverEtcdServers(ctx context.Context, kubeconfigPath string) ([]string, error) {
 	certsDir := cryptomaterial.CertsDirectory(config.DataDir)
 	etcdPeerCertDir := cryptomaterial.EtcdPeerCertDir(certsDir)
 
@@ -441,61 +441,61 @@ func discoverEtcdServers(ctx context.Context, hostname, kubeconfigPath string) (
 	if err != nil {
 		return nil, fmt.Errorf("failed to get etcd status: %w", err)
 	}
-	if st.IsLearner {
-		//TODO if its a learner I need to take the server from the current non-learner members. Use the bootstrap for that.
-		kubeconfig, err := clientcmd.LoadFromFile(kubeconfigPath)
-		if err != nil {
-			return nil, fmt.Errorf("failed to load bootstrap kubeconfig: %w", err)
-		}
 
-		if kubeconfig == nil || kubeconfig.Clusters == nil || len(kubeconfig.Clusters) == 0 {
-			return nil, fmt.Errorf("invalid bootstrap kubeconfig: no clusters found")
-		}
+	// If I am not a learner it means I am a voting member, so connecting to my own etcd instance
+	// is fine because everything is synced.
+	if !st.IsLearner {
+		return []string{"https://localhost:2379"}, nil
+	}
 
-		var etcdHost string
-		for _, cluster := range kubeconfig.Clusters {
-			etcdHost = cluster.Server
-			break
-		}
+	// If I am a learner I need to connect to a member, retrieve the list of voting
+	// members and connect to all of them.
+	kubeconfig, err := clientcmd.LoadFromFile(kubeconfigPath)
+	if err != nil {
+		return nil, fmt.Errorf("failed to load bootstrap kubeconfig: %w", err)
+	}
 
-		if etcdHost == "" {
-			return nil, fmt.Errorf("failed to extract etcd hostname from bootstrap kubeconfig")
-		}
+	if kubeconfig == nil || kubeconfig.Clusters == nil || len(kubeconfig.Clusters) == 0 {
+		return nil, fmt.Errorf("invalid bootstrap kubeconfig: no clusters found")
+	}
 
-		etcdHost = strings.TrimPrefix(etcdHost, "https://")
-		etcdHost, _, _ = net.SplitHostPort(etcdHost)
-		etcdHost = fmt.Sprintf("https://%s", net.JoinHostPort(etcdHost, "2379"))
-		client, err = clientv3.New(clientv3.Config{
-			DialTimeout: 5 * time.Second,
-			Endpoints:   []string{etcdHost},
-			TLS:         tlsConfig,
-			Context:     ctx,
-		})
-		if err != nil {
-			return nil, fmt.Errorf("failed to create etcd client: %w", err)
-		}
+	if len(kubeconfig.Clusters) > 1 {
+		return nil, fmt.Errorf("invalid bootstrap kubeconfig: multiple clusters found")
+	}
+
+	var etcdHost string
+	for _, cluster := range kubeconfig.Clusters {
+		etcdHost = cluster.Server
+		break
+	}
+
+	if etcdHost == "" {
+		return nil, fmt.Errorf("failed to extract etcd hostname from bootstrap kubeconfig")
+	}
+
+	etcdHost = strings.TrimPrefix(etcdHost, "https://")
+	etcdHost, _, _ = net.SplitHostPort(etcdHost)
+	etcdHost = fmt.Sprintf("https://%s", net.JoinHostPort(etcdHost, "2379"))
+	client, err = clientv3.New(clientv3.Config{
+		DialTimeout: 5 * time.Second,
+		Endpoints:   []string{etcdHost},
+		TLS:         tlsConfig,
+		Context:     ctx,
+	})
+	if err != nil {
+		return nil, fmt.Errorf("failed to create etcd client: %w", err)
 	}
 
 	resp, err := client.MemberList(ctx)
 	if err != nil {
 		return nil, fmt.Errorf("failed to retrieve etcd member list: %w", err)
 	}
 
-	//TODO I already know if I am a learner, I had to do this before.
-	iAmLearner := false
 	var members []string
 	for _, member := range resp.Members {
-		if member.Name == hostname && member.IsLearner {
-			iAmLearner = true
-			continue
-		}
 		if !member.IsLearner {
 			members = append(members, member.ClientURLs...)
 		}
 	}
-	if iAmLearner {
-		return members, nil
-	}
-
-	return []string{"https://localhost:2379"}, nil
+	return members, nil
 }