Skip to content

Commit

Permalink
chore: handle documents diff in apply-config dry run
Browse files Browse the repository at this point in the history 
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
~ > <editor> 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
~ >
~ >
~ >
~ > <editor> 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 <[email protected]>
  • Loading branch information
@DmitriyMV
DmitriyMV committed Jun 27, 2024
1 parent bd34f71 commit 2d054ad
Show file tree
Hide file tree
Showing 3 changed files with 111 additions and 47 deletions.
32 changes: 17 additions & 15 deletions cmd/talosctl/cmd/talos/apply-config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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,
Expand Down
45 changes: 34 additions & 11 deletions internal/app/machined/internal/server/v1alpha1/v1alpha1_server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"archive/tar"
"bufio"
"bytes"
stdcmp "cmp"
"compress/gzip"
"context"
"encoding/json"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -218,24 +221,15 @@ 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{
{
Mode: in.Mode,
ModeDetails: fmt.Sprintf(`Dry run summary:
%s (skipped in dry-run).
Config diff:
%s`, modeDetails, diff),
%s`, modeDetails, details),
},
},
}, nil
Expand Down Expand Up @@ -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 {
Expand Down
81 changes: 60 additions & 21 deletions internal/integration/api/apply-config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand Down Expand Up @@ -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 {
Expand All @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -416,21 +417,59 @@ 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)
suite.Require().NotEmpty(nodes)

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) {
Expand All @@ -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 {
Expand All @@ -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())
Expand All @@ -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
Expand Down

0 comments on commit 2d054ad

Please sign in to comment.