Skip to content

Commit 4bcaea1

Browse files
committed
feat: centralize Schematic ID computation
We compute the schematic id for a machine in two different places: in the `SchematicConfigurationController` for allocated machines, and in `MachineUpgradeStatusController` for maintenance mode machines. Centralize this computation to be done only in `SchematicConfigurationController`. Change the lifecycle of the `SchematicConfiguration` resource to be bound to a machine, not to a cluster. Signed-off-by: Utku Ozdemir <[email protected]>
1 parent 7397f14 commit 4bcaea1

File tree

17 files changed

+456
-229
lines changed

17 files changed

+456
-229
lines changed

client/api/omni/specs/omni.pb.go

Lines changed: 16 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

client/api/omni/specs/omni.proto

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1200,6 +1200,9 @@ message TalosExtensionsSpec {
12001200
message SchematicConfigurationSpec {
12011201
string schematic_id = 1;
12021202
string talos_version = 2;
1203+
1204+
// KernelArgs are the kernel args which were used to compute the SchematicId.
1205+
repeated string kernel_args = 3;
12031206
}
12041207

12051208
// ExtensionsConfigurationSpec is the desired list of extensions to be installed on the machine or the set of machines.

client/api/omni/specs/omni_vtproto.pb.go

Lines changed: 61 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

client/pkg/omni/resources/omni/cluster.go

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,16 @@ func (ClusterExtension) ResourceDefinition() meta.ResourceDefinitionSpec {
5050
Type: ClusterType,
5151
Aliases: []resource.Type{},
5252
DefaultNamespace: resources.DefaultNamespace,
53-
PrintColumns: []meta.PrintColumn{},
53+
PrintColumns: []meta.PrintColumn{
54+
{
55+
Name: "Talos Version",
56+
JSONPath: "{.talosversion}",
57+
},
58+
{
59+
Name: "Kubernetes Version",
60+
JSONPath: "{.kubernetesversion}",
61+
},
62+
},
5463
}
5564
}
5665

client/pkg/omni/resources/omni/schematic_configuration.go

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,15 @@ func (SchematicConfigurationExtension) ResourceDefinition() meta.ResourceDefinit
4343
Type: SchematicConfigurationType,
4444
Aliases: []resource.Type{},
4545
DefaultNamespace: resources.DefaultNamespace,
46-
PrintColumns: []meta.PrintColumn{},
46+
PrintColumns: []meta.PrintColumn{
47+
{
48+
Name: "Schematic ID",
49+
JSONPath: "{.schematicid}",
50+
},
51+
{
52+
Name: "Talos Version",
53+
JSONPath: "{.talosversion}",
54+
},
55+
},
4756
}
4857
}

cmd/omni/cmd/cmd.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -367,7 +367,7 @@ func defineServiceFlags() {
367367
&cmdConfig.Services.DevServerProxy.BindEndpoint,
368368
"frontend-bind",
369369
cmdConfig.Services.DevServerProxy.BindEndpoint,
370-
"proxy server which will redirect all non API requests to the definied frontend server.")
370+
"proxy server which will redirect all non API requests to the defined frontend server.")
371371
}
372372

373373
func defineAuthFlags() {
@@ -559,6 +559,12 @@ func defineStorageFlags() {
559559
cmdConfig.Storage.Default.Etcd.EmbeddedUnsafeFsync,
560560
"disable fsync in the embedded etcd server (dangerous).",
561561
)
562+
rootCmd.Flags().StringVar(
563+
&cmdConfig.Storage.Default.Etcd.EmbeddedDBPath,
564+
"etcd-embedded-db-path",
565+
cmdConfig.Storage.Default.Etcd.EmbeddedDBPath,
566+
"path to the embedded etcd database.",
567+
)
562568

563569
ensure.NoError(rootCmd.Flags().MarkHidden("etcd-embedded-unsafe-fsync"))
564570

frontend/src/api/omni/specs/omni.pb.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -796,6 +796,7 @@ export type TalosExtensionsSpec = {
796796
export type SchematicConfigurationSpec = {
797797
schematic_id?: string
798798
talos_version?: string
799+
kernel_args?: string[]
799800
}
800801

801802
export type ExtensionsConfigurationSpec = {

internal/backend/runtime/omni/controllers/omni/machineupgrade/clients.go

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,9 @@ import (
1111
"fmt"
1212
"io"
1313

14-
"github.com/siderolabs/image-factory/pkg/schematic"
1514
machineapi "github.com/siderolabs/talos/pkg/machinery/api/machine"
1615
"github.com/siderolabs/talos/pkg/machinery/client"
1716

18-
"github.com/siderolabs/omni/internal/backend/imagefactory"
1917
"github.com/siderolabs/omni/internal/backend/runtime/talos"
2018
)
2119

@@ -29,12 +27,6 @@ type TalosClient interface {
2927
UpgradeWithOptions(ctx context.Context, opts ...client.UpgradeOption) (*machineapi.UpgradeResponse, error)
3028
}
3129

32-
// ImageFactoryClient ensures that the given schematic exists in the image factory.
33-
type ImageFactoryClient interface {
34-
EnsureSchematic(ctx context.Context, inputSchematic schematic.Schematic) (imagefactory.EnsuredSchematic, error)
35-
Host() string
36-
}
37-
3830
type talosCliFactory struct{}
3931

4032
func (t *talosCliFactory) New(ctx context.Context, managementAddress string) (TalosClient, error) {

internal/backend/runtime/omni/controllers/omni/machineupgrade/status.go

Lines changed: 18 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
package machineupgrade
88

99
import (
10-
"bytes"
1110
"context"
1211
"errors"
1312
"fmt"
@@ -16,6 +15,7 @@ import (
1615

1716
"github.com/cosi-project/runtime/pkg/controller"
1817
"github.com/cosi-project/runtime/pkg/controller/generic/qtransform"
18+
"github.com/cosi-project/runtime/pkg/safe"
1919
"github.com/cosi-project/runtime/pkg/state"
2020
"github.com/siderolabs/image-factory/pkg/schematic"
2121
"github.com/siderolabs/talos/pkg/machinery/client"
@@ -32,17 +32,17 @@ import (
3232

3333
type StatusController struct {
3434
*qtransform.QController[*omni.MachineStatus, *omni.MachineUpgradeStatus]
35-
imageFactoryClient ImageFactoryClient
3635
talosClientFactory TalosClientFactory
36+
imageFactoryHost string
3737
}
3838

39-
func NewStatusController(imageFactoryClient ImageFactoryClient, talosClientFactory TalosClientFactory) *StatusController {
39+
func NewStatusController(imageFactoryHost string, talosClientFactory TalosClientFactory) *StatusController {
4040
if talosClientFactory == nil {
4141
talosClientFactory = &talosCliFactory{}
4242
}
4343

4444
ctrl := &StatusController{
45-
imageFactoryClient: imageFactoryClient,
45+
imageFactoryHost: imageFactoryHost,
4646
talosClientFactory: talosClientFactory,
4747
}
4848

@@ -58,6 +58,7 @@ func NewStatusController(imageFactoryClient ImageFactoryClient, talosClientFacto
5858
TransformFunc: ctrl.transform,
5959
},
6060
qtransform.WithExtraMappedInput[*omni.KernelArgs](qtransform.MapperSameID[*omni.MachineStatus]()),
61+
qtransform.WithExtraMappedInput[*omni.SchematicConfiguration](qtransform.MapperSameID[*omni.MachineStatus]()),
6162
)
6263

6364
return ctrl
@@ -112,33 +113,20 @@ func (ctrl *StatusController) transform(ctx context.Context, r controller.Reader
112113
return nil
113114
}
114115

115-
kernelArgsRes, err := kernelargs.GetUncached(ctx, r, ms.Metadata().ID())
116+
schematicConfiguration, err := safe.ReaderGetByID[*omni.SchematicConfiguration](ctx, r, ms.Metadata().ID())
116117
if err != nil && !state.IsNotFoundError(err) {
117-
return fmt.Errorf("error getting extra kernel args: %w", err)
118+
return fmt.Errorf("error getting schematic configuration: %w", err)
118119
}
119120

120-
kernelArgs, kernelArgsOk, err := kernelargs.Calculate(ms, kernelArgsRes)
121-
if err != nil {
122-
return fmt.Errorf("error getting extra kernel args: %w", err)
123-
}
124-
125-
if !kernelArgsOk {
121+
if schematicConfiguration == nil {
126122
status.TypedSpec().Value.Phase = specs.MachineUpgradeStatusSpec_Unknown
127-
status.TypedSpec().Value.Status = "extra kernel args are not yet initialized"
123+
status.TypedSpec().Value.Status = "schematic configuration is not available"
128124
status.TypedSpec().Value.Error = ""
129125

130-
// if extra kernel args were not initialized yet by the MachineStatusController, we cannot determine the desired set of kernel args, skip
131126
return nil
132127
}
133128

134-
desiredSchematic := currentSchematic
135-
desiredSchematic.Customization.ExtraKernelArgs = kernelArgs
136-
137-
desiredSchematicID, err := desiredSchematic.ID()
138-
if err != nil {
139-
return fmt.Errorf("failed to compute desired schematic ID: %w", err)
140-
}
141-
129+
desiredSchematicID := schematicConfiguration.TypedSpec().Value.SchematicId
142130
status.TypedSpec().Value.SchematicId = desiredSchematicID
143131

144132
talosVersion := ms.TypedSpec().Value.TalosVersion
@@ -150,7 +138,7 @@ func (ctrl *StatusController) transform(ctx context.Context, r controller.Reader
150138
return nil
151139
}
152140

153-
schematicEqual, schematicEqualWithoutKernelArgs, err := ctrl.checkSchematicEquality(currentSchematic, desiredSchematic)
141+
schematicEqual, schematicEqualWithoutKernelArgs, err := ctrl.checkSchematicEquality(currentSchematic, schematicConfiguration)
154142
if err != nil {
155143
return fmt.Errorf("failed to get schematic diff: %w", err)
156144
}
@@ -215,21 +203,6 @@ func (ctrl *StatusController) transform(ctx context.Context, r controller.Reader
215203
}
216204
}
217205

218-
ensuredSchematic, err := ctrl.imageFactoryClient.EnsureSchematic(ctx, desiredSchematic)
219-
if err != nil {
220-
return fmt.Errorf("failed to ensure schematic in image factory: %w", err)
221-
}
222-
223-
if desiredSchematicID != ensuredSchematic.FullID {
224-
logger.Error("schematic ID returned from image factory does not match desired schematic ID",
225-
zap.String("desired", desiredSchematicID), zap.String("returned", ensuredSchematic.FullID))
226-
227-
status.TypedSpec().Value.Status = ""
228-
status.TypedSpec().Value.Error = "schematic ID returned from image factory does not match desired schematic ID"
229-
230-
return nil
231-
}
232-
233206
platform := ms.TypedSpec().Value.GetPlatformMetadata().GetPlatform()
234207
if platform == "" {
235208
status.TypedSpec().Value.Status = "platform metadata is not available"
@@ -253,7 +226,7 @@ func (ctrl *StatusController) transform(ctx context.Context, r controller.Reader
253226
SecurityState: securityState,
254227
}
255228

256-
installImageStr, err := installimage.Build(ctrl.imageFactoryClient.Host(), ms.Metadata().ID(), installImage)
229+
installImageStr, err := installimage.Build(ctrl.imageFactoryHost, ms.Metadata().ID(), installImage)
257230
if err != nil {
258231
return fmt.Errorf("failed to build install image: %w", err)
259232
}
@@ -324,22 +297,17 @@ func (ctrl *StatusController) doUpgrade(ctx context.Context, machineStatus *omni
324297
return nil
325298
}
326299

327-
func (ctrl *StatusController) checkSchematicEquality(currentSchematic, desiredSchematic schematic.Schematic) (equal, equalWithoutKernelArgs bool, err error) {
328-
kernelArgsMismatch := !slices.Equal(currentSchematic.Customization.ExtraKernelArgs, desiredSchematic.Customization.ExtraKernelArgs)
329-
330-
currentSchematic.Customization.ExtraKernelArgs = desiredSchematic.Customization.ExtraKernelArgs
300+
func (ctrl *StatusController) checkSchematicEquality(currentSchematic schematic.Schematic, schematicConfig *omni.SchematicConfiguration) (equal, equalWithoutKernelArgs bool, err error) {
301+
kernelArgsMismatch := !slices.Equal(currentSchematic.Customization.ExtraKernelArgs, schematicConfig.TypedSpec().Value.KernelArgs)
331302

332-
currentMarshaled, err := currentSchematic.Marshal()
333-
if err != nil {
334-
return false, false, fmt.Errorf("failed to marshal current schematic: %w", err)
335-
}
303+
currentSchematic.Customization.ExtraKernelArgs = schematicConfig.TypedSpec().Value.KernelArgs
336304

337-
desiredMarshaled, err := desiredSchematic.Marshal()
305+
currentID, err := currentSchematic.ID()
338306
if err != nil {
339-
return false, false, fmt.Errorf("failed to marshal desired schematic: %w", err)
307+
return false, false, fmt.Errorf("failed to calculate current schematic ID: %w", err)
340308
}
341309

342-
equalWithoutKernelArgs = bytes.Equal(currentMarshaled, desiredMarshaled)
310+
equalWithoutKernelArgs = currentID == schematicConfig.TypedSpec().Value.SchematicId
343311
equal = equalWithoutKernelArgs && !kernelArgsMismatch
344312

345313
return equal, equalWithoutKernelArgs, nil

0 commit comments

Comments
 (0)