Skip to content
This repository was archived by the owner on Mar 24, 2020. It is now read-only.

only set image and user data when we actually want to provision #94

Merged
dhellmann merged 4 commits into from
Jul 2, 2019
Merged

only set image and user data when we actually want to provision #94

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
66f5599
gofmt fix
dhellmann Jul 1, 2019
0281344
refactor actuator setHostSpec tests to make it easier to add new cases
dhellmann Jul 1, 2019
5765cad
use named scenarios and subtests in setHostSpec tests
dhellmann Jul 1, 2019
899fdcf
only set image and user data when we actually want to provision
dhellmann Jul 1, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 23 additions & 9 deletions pkg/cloud/baremetal/actuators/machine/actuator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -366,22 +366,36 @@ func consumerRefMatches(consumer *corev1.ObjectReference, machine *machinev1.Mac
func (a *Actuator) setHostSpec(ctx context.Context, host *bmh.BareMetalHost, machine *machinev1.Machine,
config *bmv1alpha1.BareMetalMachineProviderSpec) error {

// We only want to update the image setting if the host does not
// already have a consumer and image.
//
// A consumer without an image is "externally provisioned" and
// assigning the image will trigger reprovisioning. Since the
// control plane nodes will be configured this way, reprovisioning
// will take them offline and try to turn them into workers.
//
// A host with an existing image is also already provisioned and
// upgrades are not supported at this time. To re-provision a
// host, we must fully deprovision it and then provision it again.
if host.Spec.ConsumerRef == nil && host.Spec.Image == nil {
host.Spec.Image = &bmh.Image{
URL: config.Image.URL,
Checksum: config.Image.Checksum,
}
host.Spec.UserData = config.UserData
if host.Spec.UserData != nil && host.Spec.UserData.Namespace == "" {
host.Spec.UserData.Namespace = machine.Namespace
}
}

host.Spec.ConsumerRef = &corev1.ObjectReference{
Kind: "Machine",
Name: machine.Name,
Namespace: machine.Namespace,
APIVersion: machine.APIVersion,
}

host.Spec.Image = &bmh.Image{
URL: config.Image.URL,
Checksum: config.Image.Checksum,
}
host.Spec.Online = true
host.Spec.UserData = config.UserData
if host.Spec.UserData != nil && host.Spec.UserData.Namespace == "" {
host.Spec.UserData.Namespace = machine.Namespace
}
return a.client.Update(ctx, host)
}

Expand Down Expand Up @@ -503,7 +517,7 @@ func (a *Actuator) nodeAddresses(host *bmh.BareMetalHost) ([]corev1.NodeAddress,

if host.Status.HardwareDetails.Hostname != "" {
addrs = append(addrs, corev1.NodeAddress{
Type: corev1.NodeHostName,
Type: corev1.NodeHostName,
Address: host.Status.HardwareDetails.Hostname,
})
}
Expand Down
235 changes: 159 additions & 76 deletions pkg/cloud/baremetal/actuators/machine/actuator_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -330,102 +330,185 @@ func TestChooseHost(t *testing.T) {

func TestSetHostSpec(t *testing.T) {
for _, tc := range []struct {
Scenario string
UserDataNamespace string
ExpectedUserDataNamespace string
Host bmh.BareMetalHost
ExpectedImage *bmh.Image
ExpectUserData bool
}{
{
Scenario: "user data has explicit alternate namespace",
UserDataNamespace: "otherns",
ExpectedUserDataNamespace: "otherns",
Host: bmh.BareMetalHost{
ObjectMeta: metav1.ObjectMeta{
Name: "host2",
Namespace: "myns",
},
},
ExpectedImage: &bmh.Image{
URL: testImageURL,
Checksum: testImageChecksumURL,
},
ExpectUserData: true,
},

{
Scenario: "user data has no namespace",
UserDataNamespace: "",
ExpectedUserDataNamespace: "myns",
Host: bmh.BareMetalHost{
ObjectMeta: metav1.ObjectMeta{
Name: "host2",
Namespace: "myns",
},
},
ExpectedImage: &bmh.Image{
URL: testImageURL,
Checksum: testImageChecksumURL,
},
ExpectUserData: true,
},
} {

// test data
config, providerSpec := newConfig(t, tc.UserDataNamespace, map[string]string{}, []bmv1alpha1.HostSelectorRequirement{})
host := bmh.BareMetalHost{
ObjectMeta: metav1.ObjectMeta{
Name: "host2",
Namespace: "myns",
{
Scenario: "externally provisioned, same machine",
UserDataNamespace: "",
ExpectedUserDataNamespace: "myns",
Host: bmh.BareMetalHost{
ObjectMeta: metav1.ObjectMeta{
Name: "host2",
Namespace: "myns",
},
Spec: bmh.BareMetalHostSpec{
ConsumerRef: &corev1.ObjectReference{
Name: "machine1",
Namespace: "myns",
Kind: "Machine",
APIVersion: machinev1.SchemeGroupVersion.String(),
},
},
},
}
machine := machinev1.Machine{
ObjectMeta: metav1.ObjectMeta{
Name: "machine1",
Namespace: "myns",
ExpectedImage: nil,
ExpectUserData: false,
},

{
Scenario: "previously provisioned, different image, unchanged",
UserDataNamespace: "",
ExpectedUserDataNamespace: "myns",
Host: bmh.BareMetalHost{
ObjectMeta: metav1.ObjectMeta{
Name: "host2",
Namespace: "myns",
},
Spec: bmh.BareMetalHostSpec{
ConsumerRef: &corev1.ObjectReference{
Name: "machine1",
Namespace: "myns",
Kind: "Machine",
APIVersion: machinev1.SchemeGroupVersion.String(),
},
Image: &bmh.Image{
URL: testImageURL + "test",
Checksum: testImageChecksumURL + "test",
},
},
},
Spec: machinev1.MachineSpec{
ProviderSpec: providerSpec,
ExpectedImage: &bmh.Image{
URL: testImageURL + "test",
Checksum: testImageChecksumURL + "test",
},
}
ExpectUserData: false,
},
} {

// test setup
scheme := runtime.NewScheme()
bmoapis.AddToScheme(scheme)
c := fakeclient.NewFakeClientWithScheme(scheme, &host)
t.Run(tc.Scenario, func(t *testing.T) {
// test data
config, providerSpec := newConfig(t, tc.UserDataNamespace, map[string]string{}, []bmv1alpha1.HostSelectorRequirement{})
machine := machinev1.Machine{
ObjectMeta: metav1.ObjectMeta{
Name: "machine1",
Namespace: "myns",
},
Spec: machinev1.MachineSpec{
ProviderSpec: providerSpec,
},
}

actuator, err := NewActuator(ActuatorParams{
Client: c,
})
if err != nil {
t.Errorf("%v", err)
return
}
// test setup
scheme := runtime.NewScheme()
bmoapis.AddToScheme(scheme)
c := fakeclient.NewFakeClientWithScheme(scheme, &tc.Host)

actuator, err := NewActuator(ActuatorParams{
Client: c,
})
if err != nil {
t.Errorf("%v", err)
return
}

// run the function
err = actuator.setHostSpec(context.TODO(), &host, &machine, config)
if err != nil {
t.Errorf("%v", err)
return
}
// run the function
err = actuator.setHostSpec(context.TODO(), &tc.Host, &machine, config)
if err != nil {
t.Errorf("%v", err)
return
}

// get the saved result
savedHost := bmh.BareMetalHost{}
err = c.Get(context.TODO(), client.ObjectKey{Name: host.Name, Namespace: host.Namespace}, &savedHost)
if err != nil {
t.Errorf("%v", err)
return
}
// get the saved result
savedHost := bmh.BareMetalHost{}
err = c.Get(context.TODO(), client.ObjectKey{Name: tc.Host.Name, Namespace: tc.Host.Namespace}, &savedHost)
if err != nil {
t.Errorf("%v", err)
return
}

// validate the result
if savedHost.Spec.ConsumerRef == nil {
t.Errorf("ConsumerRef not set")
return
}
if savedHost.Spec.ConsumerRef.Name != machine.Name {
t.Errorf("found machine ref %v", savedHost.Spec.ConsumerRef)
}
if savedHost.Spec.ConsumerRef.Namespace != machine.Namespace {
t.Errorf("found machine ref %v", savedHost.Spec.ConsumerRef)
}
if savedHost.Spec.ConsumerRef.Kind != "Machine" {
t.Errorf("found machine ref %v", savedHost.Spec.ConsumerRef)
}
if savedHost.Spec.Online != true {
t.Errorf("host not set to Online")
}
if savedHost.Spec.Image == nil {
t.Errorf("Image not set")
return
}
if savedHost.Spec.Image.URL != testImageURL {
t.Errorf("expected ImageURL %s, got %s", testImageURL, savedHost.Spec.Image.URL)
}
if savedHost.Spec.Image.Checksum != testImageChecksumURL {
t.Errorf("expected ImageChecksumURL %s, got %s", testImageChecksumURL, savedHost.Spec.Image.Checksum)
}
if savedHost.Spec.UserData == nil {
t.Errorf("UserData not set")
return
}
if savedHost.Spec.UserData.Namespace != tc.ExpectedUserDataNamespace {
t.Errorf("expected Userdata.Namespace %s, got %s", tc.ExpectedUserDataNamespace, savedHost.Spec.UserData.Namespace)
}
if savedHost.Spec.UserData.Name != testUserDataSecretName {
t.Errorf("expected Userdata.Name %s, got %s", testUserDataSecretName, savedHost.Spec.UserData.Name)
}
// validate the result
if savedHost.Spec.ConsumerRef == nil {
t.Errorf("ConsumerRef not set")
return
}
if savedHost.Spec.ConsumerRef.Name != machine.Name {
t.Errorf("found machine ref %v", savedHost.Spec.ConsumerRef)
}
if savedHost.Spec.ConsumerRef.Namespace != machine.Namespace {
t.Errorf("found machine ref %v", savedHost.Spec.ConsumerRef)
}
if savedHost.Spec.ConsumerRef.Kind != "Machine" {
t.Errorf("found machine ref %v", savedHost.Spec.ConsumerRef)
}
if savedHost.Spec.Online != true {
t.Errorf("host not set to Online")
}
if tc.ExpectedImage == nil {
if savedHost.Spec.Image != nil {
t.Errorf("Expected image %v but got %v", tc.ExpectedImage, savedHost.Spec.Image)
return
}
} else {
if *(savedHost.Spec.Image) != *(tc.ExpectedImage) {
t.Errorf("Expected image %v but got %v", tc.ExpectedImage, savedHost.Spec.Image)
return
}
}
if tc.ExpectUserData {
if savedHost.Spec.UserData == nil {
t.Errorf("UserData not set")
return
}
if savedHost.Spec.UserData.Namespace != tc.ExpectedUserDataNamespace {
t.Errorf("expected Userdata.Namespace %s, got %s", tc.ExpectedUserDataNamespace, savedHost.Spec.UserData.Namespace)
}
if savedHost.Spec.UserData.Name != testUserDataSecretName {
t.Errorf("expected Userdata.Name %s, got %s", testUserDataSecretName, savedHost.Spec.UserData.Name)
}
} else {
if savedHost.Spec.UserData != nil {
t.Errorf("did not expect user data, got %v", savedHost.Spec.UserData)
}
}
})
}
}

Expand Down