feat: always return BMC IP if found

This PR makes sure that we're attempting to patch the BMC struct twice. Once for any IP we find for the BMC info, and a second time to add the sidero user. This ensures that we'll at least populate the IP if we can. Signed-off-by: Spencer Smith <[email protected]>
siderolabs · May 26, 2021 · e558d18 · e558d18
1 parent efd300b
commit e558d18
Show file tree

Hide file tree

Showing 2 changed files with 98 additions and 38 deletions.
diff --git a/app/metal-controller-manager/cmd/agent/main.go b/app/metal-controller-manager/cmd/agent/main.go
@@ -262,11 +262,20 @@ func mainFunc() error {
 	if createResp.GetSetupBmc() {
 		log.Println("Attempting to automatically discover and configure BMC")
 
-		// nb: we don't consider failure to get BMC info a hard failure.
+		// Attempt to discover the BMC IP
+		// nb: we don't consider failure to get BMC info a hard failure
 		//     users can always patch the bmc info to the server themselves.
-		err := attemptBMCSetup(ctx, client, s)
+		err := attemptBMCIP(ctx, client, s)
 		if err != nil {
-			log.Printf("encountered error setting up BMC. skipping setup: %q", err.Error())
+			log.Printf("encountered error discovering BMC IP. skipping setup: %q", err.Error())
+		} else {
+			// Attempt to add sidero user to BMC only if we discovered the IP
+			// nb: we don't consider failure to get BMC info a hard failure.
+			//     users can always patch the bmc info to the server themselves.
+			err = attemptBMCUserSetup(ctx, client, s)
+			if err != nil {
+				log.Printf("encountered error setting up BMC user. skipping setup: %q", err.Error())
+			}
 		}
 	}
 
@@ -381,7 +390,7 @@ func main() {
 	shutdown(mainFunc())
 }
 
-func attemptBMCSetup(ctx context.Context, client api.AgentClient, s *smbios.SMBIOS) error {
+func attemptBMCIP(ctx context.Context, client api.AgentClient, s *smbios.SMBIOS) error {
 	uuid, err := s.SystemInformation().UUID()
 	if err != nil {
 		return err
@@ -408,6 +417,47 @@ func attemptBMCSetup(ctx context.Context, client api.AgentClient, s *smbios.SMBI
 	bmcIP := net.IP(ipResp.Data)
 	bmcInfo.Ip = bmcIP.String()
 
+	// Attempt to update server object
+	err = retry.Constant(5*time.Minute, retry.WithUnits(30*time.Second), retry.WithErrorLogging(true)).Retry(func() error {
+		ctx, cancel := context.WithTimeout(ctx, 30*time.Second)
+		defer cancel()
+
+		_, err = client.UpdateBMCInfo(
+			ctx,
+			&api.UpdateBMCInfoRequest{
+				Uuid:    uuid.String(),
+				BmcInfo: bmcInfo,
+			},
+		)
+
+		if err != nil {
+			return retry.ExpectedError(err)
+		}
+
+		return nil
+	})
+
+	return nil
+}
+
+func attemptBMCUserSetup(ctx context.Context, client api.AgentClient, s *smbios.SMBIOS) error {
+	uuid, err := s.SystemInformation().UUID()
+	if err != nil {
+		return err
+	}
+
+	bmcInfo := &api.BMCInfo{}
+
+	// Create "open" client
+	bmcSpec := v1alpha1.BMC{
+		Interface: "open",
+	}
+
+	ipmiClient, err := ipmi.NewClient(bmcSpec)
+	if err != nil {
+		return err
+	}
+
 	// Get user summary to see how many user slots
 	summResp, err := ipmiClient.GetUserSummary()
 	if err != nil {

diff --git a/app/metal-controller-manager/internal/server/server.go b/app/metal-controller-manager/internal/server/server.go
@@ -266,22 +266,35 @@ func (s *server) Heartbeat(ctx context.Context, in *api.HeartbeatRequest) (*api.
 }
 
 func (s *server) UpdateBMCInfo(ctx context.Context, in *api.UpdateBMCInfoRequest) (*api.UpdateBMCInfoResponse, error) {
-	if in.GetBmcInfo() != nil {
-		bmcSecretName := in.GetUuid() + "-bmc"
+	bmcInfo := in.GetBmcInfo()
+
+	// Fetch corresponding server
+	obj := &metalv1alpha1.Server{}
+
+	if err := s.c.Get(ctx, types.NamespacedName{Name: in.GetUuid()}, obj); err != nil {
+		return nil, err
+	}
+
+	// Create a BMC struct if non-existent
+	if obj.Spec.BMC == nil {
+		obj.Spec.BMC = &metalv1alpha1.BMC{}
+	}
+
+	// Update bmc info with IP if we've got it.
+	if ip := in.GetBmcInfo().GetIp(); ip != "" {
+		obj.Spec.BMC.Endpoint = ip
+	}
 
+	// Generate or update bmc secret if we have creds
+	if bmcInfo.User != "" && bmcInfo.Pass != "" {
 		// Create or update creds secret
 		credsSecret := &corev1.Secret{}
 		exists := true
 
-		// Fetch corresponding server
-		obj := &metalv1alpha1.Server{}
-
-		if err := s.c.Get(ctx, types.NamespacedName{Name: in.GetUuid()}, obj); err != nil {
-			return nil, err
-		}
-
 		// For auto-created BMC info, we will *always* drop the creds in default namespace
 		// This ensures they'll come along for the ride in a "default" clusterctl move based on our cluster templates.
+		bmcSecretName := in.GetUuid() + "-bmc"
+
 		if err := s.c.Get(ctx, types.NamespacedName{Namespace: corev1.NamespaceDefault, Name: bmcSecretName}, credsSecret); err != nil {
 			if !apierrors.IsNotFound(err) {
 				return nil, err
@@ -316,39 +329,36 @@ func (s *server) UpdateBMCInfo(ctx context.Context, in *api.UpdateBMCInfoRequest
 		}
 
 		// Update server spec with pointers to endpoint and creds secret
-
-		obj.Spec.BMC = &metalv1alpha1.BMC{
-			Endpoint: in.GetBmcInfo().GetIp(),
-			UserFrom: &metalv1alpha1.CredentialSource{
-				SecretKeyRef: &metalv1alpha1.SecretKeyRef{
-					Namespace: corev1.NamespaceDefault,
-					Name:      bmcSecretName,
-					Key:       "user",
-				},
-			},
-			PassFrom: &metalv1alpha1.CredentialSource{
-				SecretKeyRef: &metalv1alpha1.SecretKeyRef{
-					Namespace: corev1.NamespaceDefault,
-					Name:      bmcSecretName,
-					Key:       "pass",
-				},
+		obj.Spec.BMC.UserFrom = &metalv1alpha1.CredentialSource{
+			SecretKeyRef: &metalv1alpha1.SecretKeyRef{
+				Namespace: corev1.NamespaceDefault,
+				Name:      bmcSecretName,
+				Key:       "user",
 			},
 		}
 
-		log.Printf("Updating server %q with BMC info", in.GetUuid())
-
-		if err := s.c.Update(ctx, obj); err != nil {
-			return nil, err
+		obj.Spec.BMC.PassFrom = &metalv1alpha1.CredentialSource{
+			SecretKeyRef: &metalv1alpha1.SecretKeyRef{
+				Namespace: corev1.NamespaceDefault,
+				Name:      bmcSecretName,
+				Key:       "pass",
+			},
 		}
+	}
 
-		ref, err := reference.GetReference(s.scheme, obj)
-		if err != nil {
-			return nil, err
-		}
+	log.Printf("Updating server %q with BMC info", in.GetUuid())
+
+	if err := s.c.Update(ctx, obj); err != nil {
+		return nil, err
+	}
 
-		s.recorder.Event(ref, corev1.EventTypeNormal, "BMC Update", "BMC info updated via API.")
+	ref, err := reference.GetReference(s.scheme, obj)
+	if err != nil {
+		return nil, err
 	}
 
+	s.recorder.Event(ref, corev1.EventTypeNormal, "BMC Update", "BMC info updated via API.")
+
 	resp := &api.UpdateBMCInfoResponse{}
 
 	return resp, nil