From e558d189ebc1c92cb3d6c67b0833507b87a17755 Mon Sep 17 00:00:00 2001 From: Spencer Smith Date: Tue, 25 May 2021 12:50:13 -0400 Subject: [PATCH] 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 --- .../cmd/agent/main.go | 58 +++++++++++++- .../internal/server/server.go | 78 +++++++++++-------- 2 files changed, 98 insertions(+), 38 deletions(-) diff --git a/app/metal-controller-manager/cmd/agent/main.go b/app/metal-controller-manager/cmd/agent/main.go index 60b0aed49..cf0b1ae17 100644 --- 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 index f6e4ce57d..0aae7dd02 100644 --- 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