From 2d054ad3551428d8b3d93c8356b38aec7e9225eb Mon Sep 17 00:00:00 2001 From: Dmitriy Matrenichev Date: Thu, 27 Jun 2024 20:25:33 +0300 Subject: [PATCH] chore: handle documents diff in `apply-config` dry run Before this PR diff generator only diffed the v1alpha1 config and nothing else. With this PR it also takes separate docs into the account. ```shell ~ > controlplane.yaml ~ > talosctl -n talos-default-controlplane-1 apply-config --file controlplane.yaml --dry-run Dry run summary: Applied configuration without a reboot (skipped in dry-run). Config diff: No changes. Documents diff: []config.Document{ + &runtime.KmsgLogV1Alpha1{ + Meta: meta.Meta{MetaAPIVersion: "v1alpha1", MetaKind: "KmsgLogConfig"}, + MetaName: "omni-kmsg", + KmsgLogURL: s"tcp://[fdae:41e4:649b:9303::1]:8092", + }, } ~ > talosctl -n talos-default-controlplane-1 apply-config --file controlplane.yaml Applied configuration without a reboot ~ > ~ > ~ > ~ > controlplane.yaml ~ > talosctl -n talos-default-controlplane-1 apply-config --file controlplane.yaml --dry-run Dry run summary: Applied configuration without a reboot (skipped in dry-run). Config diff: No changes. Documents diff: []config.Document{ &runtime.KmsgLogV1Alpha1{Meta: {MetaAPIVersion: "v1alpha1", MetaKind: "KmsgLogConfig"}, MetaName: "omni-kmsg", KmsgLogURL: {URL: &{Scheme: "tcp", Host: "[fdae:41e4:649b:9303::1]:8092"}}}, + &network.DefaultActionConfigV1Alpha1{ + Meta: meta.Meta{MetaAPIVersion: "v1alpha1", MetaKind: "NetworkDefaultActionConfig"}, + Ingress: s"block", + }, } ``` Closes #8885 Signed-off-by: Dmitriy Matrenichev --- cmd/talosctl/cmd/talos/apply-config.go | 32 ++++---- .../server/v1alpha1/v1alpha1_server.go | 45 ++++++++--- internal/integration/api/apply-config.go | 81 ++++++++++++++----- 3 files changed, 111 insertions(+), 47 deletions(-) diff --git a/cmd/talosctl/cmd/talos/apply-config.go b/cmd/talosctl/cmd/talos/apply-config.go index 9c0d35302c..bae23bfa60 100644 --- a/cmd/talosctl/cmd/talos/apply-config.go +++ b/cmd/talosctl/cmd/talos/apply-config.go @@ -43,7 +43,7 @@ var applyConfigCmd = &cobra.Command{ RunE: func(cmd *cobra.Command, args []string) error { var ( cfgBytes []byte - e error + err error ) if len(args) > 0 { @@ -59,9 +59,9 @@ var applyConfigCmd = &cobra.Command{ } if applyConfigCmdFlags.filename != "" { - cfgBytes, e = os.ReadFile(applyConfigCmdFlags.filename) - if e != nil { - return fmt.Errorf("failed to read configuration from %q: %w", applyConfigCmdFlags.filename, e) + cfgBytes, err = os.ReadFile(applyConfigCmdFlags.filename) + if err != nil { + return fmt.Errorf("failed to read configuration from %q: %w", applyConfigCmdFlags.filename, err) } if len(cfgBytes) < 1 { @@ -74,19 +74,19 @@ var applyConfigCmd = &cobra.Command{ patches []configpatcher.Patch ) - patches, e = configpatcher.LoadPatches(applyConfigCmdFlags.patches) - if e != nil { - return e + patches, err = configpatcher.LoadPatches(applyConfigCmdFlags.patches) + if err != nil { + return err } - cfg, e = configpatcher.Apply(configpatcher.WithBytes(cfgBytes), patches) - if e != nil { - return e + cfg, err = configpatcher.Apply(configpatcher.WithBytes(cfgBytes), patches) + if err != nil { + return err } - cfgBytes, e = cfg.Bytes() - if e != nil { - return e + cfgBytes, err = cfg.Bytes() + if err != nil { + return err } } } else if applyConfigCmdFlags.Mode.Mode != helpers.InteractiveMode { @@ -108,8 +108,10 @@ var applyConfigCmd = &cobra.Command{ if len(GlobalArgs.Endpoints) > 0 { return WithClientNoNodes(func(bootstrapCtx context.Context, bootstrapClient *client.Client) error { - opts := []installer.Option{} - opts = append(opts, installer.WithBootstrapNode(bootstrapCtx, bootstrapClient, GlobalArgs.Endpoints[0]), installer.WithDryRun(applyConfigCmdFlags.dryRun)) + opts := []installer.Option{ + installer.WithBootstrapNode(bootstrapCtx, bootstrapClient, GlobalArgs.Endpoints[0]), + installer.WithDryRun(applyConfigCmdFlags.dryRun), + } conn, err := installer.NewConnection( ctx, diff --git a/internal/app/machined/internal/server/v1alpha1/v1alpha1_server.go b/internal/app/machined/internal/server/v1alpha1/v1alpha1_server.go index da8cf87707..4000dabd9e 100644 --- a/internal/app/machined/internal/server/v1alpha1/v1alpha1_server.go +++ b/internal/app/machined/internal/server/v1alpha1/v1alpha1_server.go @@ -8,6 +8,7 @@ import ( "archive/tar" "bufio" "bytes" + stdcmp "cmp" "compress/gzip" "context" "encoding/json" @@ -76,6 +77,8 @@ import ( "github.com/siderolabs/talos/pkg/machinery/api/storage" timeapi "github.com/siderolabs/talos/pkg/machinery/api/time" clientconfig "github.com/siderolabs/talos/pkg/machinery/client/config" + "github.com/siderolabs/talos/pkg/machinery/config" + docscfg "github.com/siderolabs/talos/pkg/machinery/config/config" "github.com/siderolabs/talos/pkg/machinery/config/configloader" "github.com/siderolabs/talos/pkg/machinery/config/generate/secrets" machinetype "github.com/siderolabs/talos/pkg/machinery/config/machine" @@ -218,15 +221,7 @@ func (s *Server) ApplyConfiguration(ctx context.Context, in *machine.ApplyConfig } if in.DryRun { - var config interface{} - if s.Controller.Runtime().Config() != nil { - config = s.Controller.Runtime().ConfigContainer().RawV1Alpha1() - } - - diff := cmp.Diff(config, cfgProvider.RawV1Alpha1(), cmp.AllowUnexported(v1alpha1.InstallDiskSizeMatcher{})) - if diff == "" { - diff = "No changes." - } + details := generateDiff(s.Controller.Runtime(), cfgProvider) return &machine.ApplyConfigurationResponse{ Messages: []*machine.ApplyConfiguration{ @@ -234,8 +229,7 @@ func (s *Server) ApplyConfiguration(ctx context.Context, in *machine.ApplyConfig Mode: in.Mode, ModeDetails: fmt.Sprintf(`Dry run summary: %s (skipped in dry-run). -Config diff: -%s`, modeDetails, diff), +%s`, modeDetails, details), }, }, }, nil @@ -301,6 +295,35 @@ Config diff: }, nil } +func generateDiff(r runtime.Runtime, provider config.Provider) string { + var cfg *v1alpha1.Config + + if r.Config() != nil { + cfg = r.ConfigContainer().RawV1Alpha1() + } + + v1alpha1Diff := cmp.Diff(cfg, provider.RawV1Alpha1(), cmp.AllowUnexported(v1alpha1.InstallDiskSizeMatcher{})) + if v1alpha1Diff == "" { + v1alpha1Diff = "No changes." + } + + origDocs := slices.DeleteFunc(r.ConfigContainer().Documents(), func(doc docscfg.Document) bool { return doc.Kind() == v1alpha1.Version }) + newDocs := slices.DeleteFunc(provider.Documents(), func(doc docscfg.Document) bool { return doc.Kind() == v1alpha1.Version }) + + slices.SortStableFunc(origDocs, func(a, b docscfg.Document) int { return stdcmp.Compare(a.Kind(), b.Kind()) }) + slices.SortStableFunc(newDocs, func(a, b docscfg.Document) int { return stdcmp.Compare(a.Kind(), b.Kind()) }) + + documentsDiff := cmp.Diff(origDocs, newDocs) + if documentsDiff == "" { + documentsDiff = "No changes." + } + + return fmt.Sprintf(`Config diff: +%s +Documents diff: +%s`, v1alpha1Diff, documentsDiff) +} + // GenerateConfiguration implements the machine.MachineServer interface. func (s *Server) GenerateConfiguration(ctx context.Context, in *machine.GenerateConfigurationRequest) (reply *machine.GenerateConfigurationResponse, err error) { if s.Controller.Runtime().Config().Machine().Type() == machinetype.TypeWorker { diff --git a/internal/integration/api/apply-config.go b/internal/integration/api/apply-config.go index b3b22a20a7..5497e43dd7 100644 --- a/internal/integration/api/apply-config.go +++ b/internal/integration/api/apply-config.go @@ -8,12 +8,14 @@ package api import ( "context" + "net/url" "os" - "sort" + "slices" "testing" "time" "github.com/cosi-project/runtime/pkg/safe" + "github.com/siderolabs/gen/ensure" "github.com/siderolabs/go-pointer" "github.com/siderolabs/go-retry/retry" "google.golang.org/grpc/codes" @@ -23,7 +25,9 @@ import ( machineapi "github.com/siderolabs/talos/pkg/machinery/api/machine" "github.com/siderolabs/talos/pkg/machinery/client" "github.com/siderolabs/talos/pkg/machinery/config" + "github.com/siderolabs/talos/pkg/machinery/config/container" "github.com/siderolabs/talos/pkg/machinery/config/machine" + "github.com/siderolabs/talos/pkg/machinery/config/types/runtime" "github.com/siderolabs/talos/pkg/machinery/config/types/v1alpha1" "github.com/siderolabs/talos/pkg/machinery/constants" mc "github.com/siderolabs/talos/pkg/machinery/resources/config" @@ -88,14 +92,13 @@ func (suite *ApplyConfigSuite) TestApply() { suite.WaitForBootDone(suite.ctx) - sort.Strings(nodes) + slices.Sort(nodes) node := nodes[0] - nodeCtx := client.WithNode(suite.ctx, node) provider, err := suite.ReadConfigFromNode(nodeCtx) - suite.Assert().Nilf(err, "failed to read existing config from node %q: %w", node, err) + suite.Assert().NoErrorf(err, "failed to read existing config from node %q: %w", node, err) cfgDataOut := suite.PatchV1Alpha1Config(provider, func(cfg *v1alpha1.Config) { if cfg.MachineConfig.MachineSysctls == nil { @@ -115,7 +118,7 @@ func (suite *ApplyConfigSuite) TestApply() { ) if err != nil { // It is expected that the connection will EOF here, so just log the error - suite.Assert().Nilf(err, "failed to apply configuration (node %q): %w", node, err) + suite.Assert().NoErrorf(err, "failed to apply configuration (node %q): %w", node, err) } return nil @@ -125,7 +128,7 @@ func (suite *ApplyConfigSuite) TestApply() { // Verify configuration change var newProvider config.Provider - suite.Require().Nilf( + suite.Require().NoErrorf( retry.Constant(time.Minute, retry.WithUnits(time.Second)).Retry( func() error { newProvider, err = suite.ReadConfigFromNode(nodeCtx) @@ -300,7 +303,7 @@ func (suite *ApplyConfigSuite) TestApplyConfigRotateEncryptionSecrets() { ) if err != nil { // It is expected that the connection will EOF here, so just log the error - suite.Assert().Nilf(err, "failed to apply configuration (node %q): %w", node, err) + suite.Assert().Errorf(err, "failed to apply configuration (node %q): %w", node, err) } return nil @@ -312,7 +315,7 @@ func (suite *ApplyConfigSuite) TestApplyConfigRotateEncryptionSecrets() { // Verify configuration change var newProvider config.Provider - suite.Require().Nilf( + suite.Require().Errorf( retry.Constant(time.Minute, retry.WithUnits(time.Second)).Retry( func() error { newProvider, err = suite.ReadConfigFromNode(nodeCtx) @@ -355,14 +358,13 @@ func (suite *ApplyConfigSuite) TestApplyNoReboot() { suite.WaitForBootDone(suite.ctx) - sort.Strings(nodes) + slices.Sort(nodes) node := nodes[0] - nodeCtx := client.WithNode(suite.ctx, node) provider, err := suite.ReadConfigFromNode(nodeCtx) - suite.Require().Nilf(err, "failed to read existing config from node %q: %s", node, err) + suite.Require().NoErrorf(err, "failed to read existing config from node %q: %s", node, err) cfgDataOut := suite.PatchV1Alpha1Config(provider, func(cfg *v1alpha1.Config) { // this won't be possible without a reboot @@ -387,14 +389,13 @@ func (suite *ApplyConfigSuite) TestApplyDryRun() { suite.WaitForBootDone(suite.ctx) - sort.Strings(nodes) + slices.Sort(nodes) node := nodes[0] - nodeCtx := client.WithNode(suite.ctx, node) provider, err := suite.ReadConfigFromNode(nodeCtx) - suite.Require().Nilf(err, "failed to read existing config from node %q: %s", node, err) + suite.Require().NoErrorf(err, "failed to read existing config from node %q: %s", node, err) cfgDataOut := suite.PatchV1Alpha1Config(provider, func(cfg *v1alpha1.Config) { // this won't be possible without a reboot @@ -416,10 +417,49 @@ func (suite *ApplyConfigSuite) TestApplyDryRun() { }, ) - suite.Require().Nilf(err, "failed to apply configuration (node %q): %s", node, err) + suite.Require().NoErrorf(err, "failed to apply configuration (node %q): %s", node, err) suite.Assert().Contains(reply.Messages[0].ModeDetails, "Dry run summary") } +// TestApplyDryRunDocuments verifies the apply config API with multi doc and dry run enabled. +func (suite *ApplyConfigSuite) TestApplyDryRunDocuments() { + nodes := suite.DiscoverNodeInternalIPsByType(suite.ctx, machine.TypeWorker) + suite.Require().NotEmpty(nodes) + + suite.WaitForBootDone(suite.ctx) + + slices.Sort(nodes) + + node := nodes[0] + nodeCtx := client.WithNode(suite.ctx, node) + + provider, err := suite.ReadConfigFromNode(nodeCtx) + suite.Require().NoErrorf(err, "failed to read existing config from node %q: %s", node, err) + + kmsg := runtime.NewKmsgLogV1Alpha1() + kmsg.MetaName = "omni-kmsg" + kmsg.KmsgLogURL.URL = ensure.Value(url.Parse("tcp://[fdae:41e4:649b:9303::1]:8092")) + + cont, err := container.New(provider.RawV1Alpha1(), kmsg) + suite.Require().NoErrorf(err, "failed to create container: %s", err) + + cfgDataOut, err := cont.Bytes() + suite.Require().NoErrorf(err, "failed to marshal container: %s", err) + + reply, err := suite.Client.ApplyConfiguration( + nodeCtx, &machineapi.ApplyConfigurationRequest{ + Data: cfgDataOut, + Mode: machineapi.ApplyConfigurationRequest_AUTO, + DryRun: true, + }, + ) + + suite.Require().NoErrorf(err, "failed to apply configuration (node %q): %s", node, err) + suite.Assert().Contains(reply.Messages[0].ModeDetails, "Dry run summary") + suite.Assert().Contains(reply.Messages[0].ModeDetails, "omni-kmsg") + suite.Assert().Contains(reply.Messages[0].ModeDetails, "tcp://[fdae:41e4:649b:9303::1]:8092") +} + // TestApplyTry applies the config in try mode with a short timeout. func (suite *ApplyConfigSuite) TestApplyTry() { nodes := suite.DiscoverNodeInternalIPsByType(suite.ctx, machine.TypeWorker) @@ -427,10 +467,9 @@ func (suite *ApplyConfigSuite) TestApplyTry() { suite.WaitForBootDone(suite.ctx) - sort.Strings(nodes) + slices.Sort(nodes) node := nodes[0] - nodeCtx := client.WithNode(suite.ctx, node) getMachineConfig := func(ctx context.Context) (*mc.MachineConfig, error) { @@ -443,7 +482,7 @@ func (suite *ApplyConfigSuite) TestApplyTry() { } provider, err := getMachineConfig(nodeCtx) - suite.Require().Nilf(err, "failed to read existing config from node %q: %s", node, err) + suite.Require().NoErrorf(err, "failed to read existing config from node %q: %s", node, err) cfgDataOut := suite.PatchV1Alpha1Config(provider.Provider(), func(cfg *v1alpha1.Config) { if cfg.MachineConfig.MachineNetwork == nil { @@ -465,10 +504,10 @@ func (suite *ApplyConfigSuite) TestApplyTry() { TryModeTimeout: durationpb.New(time.Second * 1), }, ) - suite.Assert().Nilf(err, "failed to apply configuration (node %q): %s", node, err) + suite.Assert().NoErrorf(err, "failed to apply configuration (node %q): %s", node, err) provider, err = getMachineConfig(nodeCtx) - suite.Require().Nilf(err, "failed to read existing config from node %q: %w", node, err) + suite.Require().NoErrorf(err, "failed to read existing config from node %q: %w", node, err) suite.Assert().NotNil(provider.Config().Machine().Network()) suite.Assert().NotNil(provider.Config().Machine().Network().Devices()) @@ -487,7 +526,7 @@ func (suite *ApplyConfigSuite) TestApplyTry() { for range 100 { provider, err = getMachineConfig(nodeCtx) - suite.Assert().Nilf(err, "failed to read existing config from node %q: %s", node, err) + suite.Assert().NoErrorf(err, "failed to read existing config from node %q: %s", node, err) if provider.Config().Machine().Network() == nil { return