From 7376f34e823f6399ed2c66ae1296a8a47a0a00ef Mon Sep 17 00:00:00 2001
From: Utku Ozdemir <utku.ozdemir@siderolabs.com>
Date: Thu, 14 Mar 2024 10:59:10 +0100
Subject: [PATCH] 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 siderolabs/talos#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 <utku.ozdemir@siderolabs.com>
---
 .../runtime/maintenance_service.go            |  6 +++++
 .../runtime/maintenance_service_test.go       | 27 ++++++++++++++++++-
 2 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/internal/app/machined/pkg/controllers/runtime/maintenance_service.go b/internal/app/machined/pkg/controllers/runtime/maintenance_service.go
index 7a0f7fcb84..2e83d62a4c 100644
--- 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
index a6aab9de44..ed86430e24 100644
--- 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://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://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 {