Skip to content

Commit

Permalink
fix: break the link of metadata server to Talos machine config
Browse files Browse the repository at this point in the history
Previous PR used Talos machinery to load the config, and then generate
config patch to patch the config. This worked fine until Talos started
validating loaded machine config for 'unknown fields'. This breaks this
flow, as Omni Talos machinery version should be >= Talos version.

Rewriting the code to skip loading using Talos machinery, also handle
cases of broken config without `machine` or `machine.kubelet` sections.
These will probably be invalid anyways, but we let Talos report it
instead of panic in Sidero.

Fixes #1076

Also use Talos machinery configpatcher to reduce code duplication.

And, last but not the least, add a unit-test for the metadata server,
which covers all major code paths.

```
ok  	github.com/siderolabs/sidero/app/sidero-controller-manager/internal/metadata	0.022s	coverage: 72.8% of statements
```

Signed-off-by: Andrey Smirnov <[email protected]>
  • Loading branch information
smira committed Apr 10, 2023
1 parent ef65ff0 commit 24e1893
Show file tree
Hide file tree
Showing 9 changed files with 498 additions and 99 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ func (r *ServerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr

// Talos installation was successful, so mark the server as PXE booted.
if conditions.IsTrue(serverBinding, infrav1.TalosInstalledCondition) {
conditions.MarkTrue(serverBinding, metalv1.ConditionPXEBooted)
conditions.MarkTrue(&s, metalv1.ConditionPXEBooted)
}
}
}
Expand Down
29 changes: 0 additions & 29 deletions app/sidero-controller-manager/internal/ipxe/ipxe_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,7 @@ import (
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
"sigs.k8s.io/cluster-api/util/conditions"
"sigs.k8s.io/cluster-api/util/patch"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/healthz"

Expand Down Expand Up @@ -238,20 +236,6 @@ func ipxeHandler(w http.ResponseWriter, r *http.Request) {

return
}

// This code is left here only for backward compatibility with Talos <= v0.13.
if !strings.HasPrefix(env.ObjectMeta.Name, "agent") {
// Do not mark as PXE booted here if SideroLink events are available and Talos installation is in progress.
// SideroLink events handler will mark the machine with TalosInstalledCondition condition,
// then server controller will reconcile this status and mark server as PXEBooted.
if conditions.Has(serverBinding, infrav1.TalosInstalledCondition) {
return
}

if err = markAsPXEBooted(ctx, server); err != nil {
log.Printf("error marking server as PXE booted: %s", err)
}
}
}

func getBootFromDiskMethod(ctx context.Context, server *metalv1.Server, serverBinding *infrav1.ServerBinding) (siderotypes.BootFromDisk, error) {
Expand Down Expand Up @@ -540,19 +524,6 @@ outer:
}
}

func markAsPXEBooted(ctx context.Context, server *metalv1.Server) error {
patchHelper, err := patch.NewHelper(server, c)
if err != nil {
return err
}

conditions.MarkTrue(server, metalv1.ConditionPXEBooted)

return patchHelper.Patch(ctx, server, patch.WithOwnedConditions{
Conditions: []clusterv1.ConditionType{metalv1.ConditionPXEBooted},
})
}

func Check(addr string) healthz.Checker {
return func(_ *http.Request) error {
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
Expand Down
270 changes: 270 additions & 0 deletions app/sidero-controller-manager/internal/metadata/fixture_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,270 @@
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this
// file, You can obtain one at http://mozilla.org/MPL/2.0/.

package metadata_test

import (
"fmt"

corev1 "k8s.io/api/core/v1"
v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/utils/pointer"
capiv1 "sigs.k8s.io/cluster-api/api/v1beta1"
"sigs.k8s.io/controller-runtime/pkg/client"

infrav1 "github.com/siderolabs/sidero/app/caps-controller-manager/api/v1alpha3"
metalv1 "github.com/siderolabs/sidero/app/sidero-controller-manager/api/v1alpha2"
)

func fixture() []client.Object {
var objects []client.Object

for _, fixture := range []func() []client.Object{
fixture1,
fixture2,
fixture3,
fixture4,
fixture5,
} {
objects = append(objects, fixture()...)
}

return objects
}

// fixture1 creates a server without config patches.
func fixture1() []client.Object {
return fixtureSimple("0000-1111-2222", 1, `
version: v1alpha1
machine:
kubelet: {}
`)
}

// fixture2 creates a server with Server-level config patches.
func fixture2() []client.Object {
return []client.Object{
&infrav1.ServerBinding{
ObjectMeta: metav1.ObjectMeta{
Name: "1111-2222-3333",
},
Spec: infrav1.ServerBindingSpec{
MetalMachineRef: corev1.ObjectReference{
Name: "metal-machine-2",
},
},
},
&infrav1.MetalMachine{
ObjectMeta: metav1.ObjectMeta{
Name: "metal-machine-2",
OwnerReferences: []metav1.OwnerReference{
{
APIVersion: "cluster.x-k8s.io/v1beta1",
Kind: "Machine",
Name: "machine-2",
},
},
},
Spec: infrav1.MetalMachineSpec{},
},
&capiv1.Machine{
ObjectMeta: metav1.ObjectMeta{
Name: "machine-2",
},
Spec: capiv1.MachineSpec{
Bootstrap: capiv1.Bootstrap{
DataSecretName: pointer.String("bootstrap2"),
},
},
},
&metalv1.Server{
ObjectMeta: metav1.ObjectMeta{
Name: "1111-2222-3333",
},
Spec: metalv1.ServerSpec{
ConfigPatches: []metalv1.ConfigPatches{
{
Op: "add",
Path: "/machine/network",
Value: v1.JSON{
Raw: []byte(`{"hostname":"example2"}`),
},
},
},
},
},
&corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "bootstrap2",
},
Data: map[string][]byte{
"value": []byte(`
version: v1alpha1
machine:
kubelet:
extraArgs:
foo: bar
`),
},
},
}
}

// fixture3 creates a server with Server- & ServerClass-level config patches.
func fixture3() []client.Object {
return []client.Object{
&infrav1.ServerBinding{
ObjectMeta: metav1.ObjectMeta{
Name: "2222-3333-4444",
},
Spec: infrav1.ServerBindingSpec{
MetalMachineRef: corev1.ObjectReference{
Name: "metal-machine-3",
},
ServerClassRef: &corev1.ObjectReference{
Name: "server-class-3",
},
},
},
&infrav1.MetalMachine{
ObjectMeta: metav1.ObjectMeta{
Name: "metal-machine-3",
OwnerReferences: []metav1.OwnerReference{
{
APIVersion: "cluster.x-k8s.io/v1beta1",
Kind: "Machine",
Name: "machine-3",
},
},
},
Spec: infrav1.MetalMachineSpec{},
},
&capiv1.Machine{
ObjectMeta: metav1.ObjectMeta{
Name: "machine-3",
},
Spec: capiv1.MachineSpec{
Bootstrap: capiv1.Bootstrap{
DataSecretName: pointer.String("bootstrap3"),
},
},
},
&metalv1.ServerClass{
ObjectMeta: metav1.ObjectMeta{
Name: "server-class-3",
},
Spec: metalv1.ServerClassSpec{
ConfigPatches: []metalv1.ConfigPatches{
{
Op: "add",
Path: "/machine/network",
Value: v1.JSON{
Raw: []byte(`{"hostname":"invalid3"}`),
},
},
},
},
},
&metalv1.Server{
ObjectMeta: metav1.ObjectMeta{
Name: "2222-3333-4444",
},
Spec: metalv1.ServerSpec{
ConfigPatches: []metalv1.ConfigPatches{
{
Op: "replace",
Path: "/machine/network/hostname",
Value: v1.JSON{
Raw: []byte(`"example3"`),
},
},
},
},
},
&corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "bootstrap3",
},
Data: map[string][]byte{
"value": []byte(`
version: v1alpha1
machine:
kubelet:
extraArgs:
node-labels: foo=bar
`),
},
},
}
}

// fixture4 creates a server machine config without machine.kubelet.
func fixture4() []client.Object {
return fixtureSimple("4444-5555-6666", 4, `
version: v1alpha1
machine:
unsupported: {} # this is not supported by Talos, but should be passed through
`)
}

// fixture5 creates a server machine config without machine.
func fixture5() []client.Object {
return fixtureSimple("5555-6666-7777", 5, `
version: v1alpha1
cluster: {}
`)
}

func fixtureSimple(uuid string, index int, config string) []client.Object {
return []client.Object{
&infrav1.ServerBinding{
ObjectMeta: metav1.ObjectMeta{
Name: uuid,
},
Spec: infrav1.ServerBindingSpec{
MetalMachineRef: corev1.ObjectReference{
Name: fmt.Sprintf("metal-machine-%d", index),
},
},
},
&infrav1.MetalMachine{
ObjectMeta: metav1.ObjectMeta{
Name: fmt.Sprintf("metal-machine-%d", index),
OwnerReferences: []metav1.OwnerReference{
{
APIVersion: "cluster.x-k8s.io/v1beta1",
Kind: "Machine",
Name: fmt.Sprintf("machine-%d", index),
},
},
},
Spec: infrav1.MetalMachineSpec{},
},
&capiv1.Machine{
ObjectMeta: metav1.ObjectMeta{
Name: fmt.Sprintf("machine-%d", index),
},
Spec: capiv1.MachineSpec{
Bootstrap: capiv1.Bootstrap{
DataSecretName: pointer.String(fmt.Sprintf("bootstrap%d", index)),
},
},
},
&metalv1.Server{
ObjectMeta: metav1.ObjectMeta{
Name: uuid,
},
Spec: metalv1.ServerSpec{},
},
&corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: fmt.Sprintf("bootstrap%d", index),
},
Data: map[string][]byte{
"value": []byte(config),
},
},
}
}
Loading

0 comments on commit 24e1893

Please sign in to comment.