fix: remove maintenance config when maintenance service is shut down

We now remove the machine config with the id `maintenance` when we are done with it - when the maintenance service is shut down. Closes #8424, where in some configurations there would be machine configs with both `v1alpha1` and `maintenance` IDs present, causing the `talosctl edit machineconfig` to loop twice and causing confusion. Signed-off-by: Utku Ozdemir <[email protected]>
siderolabs · Mar 14, 2024 · 7376f34 · 7376f34
1 parent 952801d
commit 7376f34
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 1 deletion.
diff --git a/internal/app/machined/pkg/controllers/runtime/maintenance_service.go b/internal/app/machined/pkg/controllers/runtime/maintenance_service.go
@@ -111,6 +111,12 @@ func (ctrl *MaintenanceServiceController) Run(ctx context.Context, r controller.
 			lastReachableAddresses = nil
 			lastListenAddress = ""
 		}
+
+		// clean up maintenance machine config, as we are done with the maintenance service
+		err := r.Destroy(ctx, config.NewMachineConfigWithID(nil, config.MaintenanceID).Metadata())
+		if err != nil && !state.IsNotFoundError(err) {
+			logger.Error("failed to destroy maintenance machine config", zap.String("id", config.MaintenanceID), zap.Error(err))
+		}
 	}
 
 	defer shutdownServer(context.Background())

diff --git a/internal/app/machined/pkg/controllers/runtime/maintenance_service_test.go b/internal/app/machined/pkg/controllers/runtime/maintenance_service_test.go
@@ -25,9 +25,12 @@ import (
 	runtimectrl "github.com/siderolabs/talos/internal/app/machined/pkg/controllers/runtime"
 	"github.com/siderolabs/talos/internal/app/machined/pkg/controllers/secrets"
 	talosruntime "github.com/siderolabs/talos/internal/app/machined/pkg/runtime"
+	"github.com/siderolabs/talos/internal/app/machined/pkg/runtime/v1alpha1/platform/metal"
 	"github.com/siderolabs/talos/internal/app/maintenance"
+	machineapi "github.com/siderolabs/talos/pkg/machinery/api/machine"
 	"github.com/siderolabs/talos/pkg/machinery/client"
 	"github.com/siderolabs/talos/pkg/machinery/config"
+	configres "github.com/siderolabs/talos/pkg/machinery/resources/config"
 	"github.com/siderolabs/talos/pkg/machinery/resources/hardware"
 	"github.com/siderolabs/talos/pkg/machinery/resources/network"
 	"github.com/siderolabs/talos/pkg/machinery/resources/runtime"
@@ -114,6 +117,25 @@ func (suite *MaintenanceServiceSuite) TestRunService() {
 	_, err = mc.Version(suite.Ctx())
 	suite.Require().ErrorContains(err, "API is not implemented in maintenance mode")
 
+	// apply partial machine config
+	_, err = mc.ApplyConfiguration(suite.Ctx(), &machineapi.ApplyConfigurationRequest{
+		Data: []byte(`
+apiVersion: v1alpha1
+kind: KmsgLogConfig
+name: test
+url: "tcp:https://127.0.0.42:1234"
+`),
+	})
+	suite.Require().NoError(err)
+
+	// assert that the config with the maintenance ID is created
+	rtestutils.AssertResource[*configres.MachineConfig](suite.Ctx(), suite.T(), suite.State(), configres.MaintenanceID, func(r *configres.MachineConfig, assertion *assert.Assertions) {
+		configBytes, configBytesErr := r.Container().Bytes()
+		assertion.NoError(configBytesErr)
+
+		assertion.Contains(string(configBytes), "tcp:https://127.0.0.42:1234")
+	})
+
 	suite.Require().NoError(mc.Close())
 
 	// change the listen address
@@ -168,6 +190,9 @@ func (suite *MaintenanceServiceSuite) TestRunService() {
 
 	_, err = net.Dial("tcp", maintenanceConfig.TypedSpec().ListenAddress)
 	suite.Require().ErrorContains(err, "connection refused")
+
+	// assert that the maintenance service is removed from the config after the service was shut down
+	rtestutils.AssertNoResource[*configres.MachineConfig](suite.Ctx(), suite.T(), suite.State(), configres.MaintenanceID)
 }
 
 type mockController struct {
@@ -242,7 +267,7 @@ func (mock mockController) IsBootstrapAllowed() bool {
 }
 
 func (mock mockState) Platform() talosruntime.Platform {
-	return nil
+	return &metal.Metal{} // required for ApplyConfiguration to not fail
 }
 
 func (mock mockState) Machine() talosruntime.MachineState {