Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Bug 1843950: remove infinite reconcile loop when fetching status annotation #75

Merged
merged 4 commits into from
Jun 5, 2020
Merged

Bug 1843950: remove infinite reconcile loop when fetching status annotation #75

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
7f3d1f8
fix typo in variable name in controller tests
dhellmann Jun 4, 2020
b6a368b
create reusable functions for managing the status annotation content
dhellmann Jun 4, 2020
7ef20ab
ensure last updated is set when extracting status from annotation
dhellmann Jun 4, 2020
acdebc6
fix typo in output message for get-hardware-details
dhellmann Jun 4, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/get-hardware-details/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ func main() {
IronicInspectorEndpoint: os.Args[1],
})
if err != nil {
fmt.Printf("could not get inpsector client: %s", err)
fmt.Printf("could not get inspector client: %s", err)
os.Exit(1)
}

Expand Down
31 changes: 27 additions & 4 deletions pkg/controller/baremetalhost/baremetalhost_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,13 @@ func (r *ReconcileBareMetalHost) Reconcile(request reconcile.Request) (result re
objStatus, err := r.getHostStatusFromAnnotation(host)
if err == nil && objStatus != nil {
host.Status = *objStatus
if host.Status.LastUpdated.IsZero() {
// Ensure the LastUpdated timestamp in set to avoid
// infinite loops if the annotation only contained
// part of the status information.
t := metav1.Now()
host.Status.LastUpdated = &t
}
errStatus := r.client.Status().Update(context.TODO(), host)
if errStatus != nil {
return reconcile.Result{}, errors.Wrap(err, "Could not restore status from annotation")
Expand Down Expand Up @@ -829,7 +836,7 @@ func (r *ReconcileBareMetalHost) saveHostAnnotation(host *metal3v1alpha1.BareMet
}

delete(host.Annotations, metal3v1alpha1.StatusAnnotation)
newAnnotation, err := json.Marshal(host.Status)
newAnnotation, err := marshalStatusAnnotation(&host.Status)
if err != nil {
return err
}
Expand All @@ -840,16 +847,32 @@ func (r *ReconcileBareMetalHost) saveHostAnnotation(host *metal3v1alpha1.BareMet
return r.client.Update(context.TODO(), host.DeepCopy())
}

func marshalStatusAnnotation(status *metal3v1alpha1.BareMetalHostStatus) ([]byte, error) {
newAnnotation, err := json.Marshal(status)
if err != nil {
return []byte{}, errors.Wrap(err, "failed to marshall status annotation")
}
return newAnnotation, nil
}

func unmarshalStatusAnnotation(content []byte) (*metal3v1alpha1.BareMetalHostStatus, error) {
objStatus := &metal3v1alpha1.BareMetalHostStatus{}
if err := json.Unmarshal(content, objStatus); err != nil {
return nil, errors.Wrap(err, "Failed to fetch Status from annotation")
}
return objStatus, nil
}

// extract host from Status annotation
func (r *ReconcileBareMetalHost) getHostStatusFromAnnotation(host *metal3v1alpha1.BareMetalHost) (*metal3v1alpha1.BareMetalHostStatus, error) {
annotations := host.GetAnnotations()
content := []byte(annotations[metal3v1alpha1.StatusAnnotation])
if annotations[metal3v1alpha1.StatusAnnotation] == "" {
return nil, nil
}
objStatus := &metal3v1alpha1.BareMetalHostStatus{}
if err := json.Unmarshal(content, objStatus); err != nil {
return nil, errors.Wrap(err, "Failed to fetch Status from annotation")
objStatus, err := unmarshalStatusAnnotation(content)
if err != nil {
return nil, err
}
return objStatus, nil
}
Expand Down
45 changes: 41 additions & 4 deletions pkg/controller/baremetalhost/baremetalhost_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import (
const (
namespace string = "test-namespace"
defaultSecretName string = "bmc-creds-valid"
statusAnnotaion string = `{"operationalStatus":"OK","lastUpdated":"2020-04-15T15:00:50Z","hardwareProfile":"StatusProfile","hardware":{"systemVendor":{"manufacturer":"QEMU","productName":"Standard PC (Q35 + ICH9, 2009)","serialNumber":""},"firmware":{"bios":{"date":"","vendor":"","version":""}},"ramMebibytes":4096,"nics":[{"name":"eth0","model":"0x1af4 0x0001","mac":"00:b7:8b:bb:3d:f6","ip":"172.22.0.64","speedGbps":0,"vlanId":0,"pxe":true},{"name":"eth1","model":"0x1af4 0x0001","mac":"00:b7:8b:bb:3d:f8","ip":"192.168.111.20","speedGbps":0,"vlanId":0,"pxe":false}],"storage":[{"name":"/dev/sda","rotational":true,"sizeBytes":53687091200,"vendor":"QEMU","model":"QEMU HARDDISK","serialNumber":"drive-scsi0-0-0-0","hctl":"6:0:0:0"}],"cpu":{"arch":"x86_64","model":"Intel Xeon E3-12xx v2 (IvyBridge)","clockMegahertz":2494.224,"flags":["aes","apic","arat","avx","clflush","cmov","constant_tsc","cx16","cx8","de","eagerfpu","ept","erms","f16c","flexpriority","fpu","fsgsbase","fxsr","hypervisor","lahf_lm","lm","mca","mce","mmx","msr","mtrr","nopl","nx","pae","pat","pclmulqdq","pge","pni","popcnt","pse","pse36","rdrand","rdtscp","rep_good","sep","smep","sse","sse2","sse4_1","sse4_2","ssse3","syscall","tpr_shadow","tsc","tsc_adjust","tsc_deadline_timer","vme","vmx","vnmi","vpid","x2apic","xsave","xsaveopt","xtopology"],"count":4},"hostname":"node-0"},"provisioning":{"state":"provisioned","ID":"8a0ede17-7b87-44ac-9293-5b7d50b94b08","image":{"url":"bar","checksum":""}},"goodCredentials":{"credentials":{"name":"node-0-bmc-secret","namespace":"metal3"},"credentialsVersion":"879"},"triedCredentials":{"credentials":{"name":"node-0-bmc-secret","namespace":"metal3"},"credentialsVersion":"879"},"errorMessage":"","poweredOn":true,"operationHistory":{"register":{"start":"2020-04-15T12:06:26Z","end":"2020-04-15T12:07:12Z"},"inspect":{"start":"2020-04-15T12:07:12Z","end":"2020-04-15T12:09:29Z"},"provision":{"start":null,"end":null},"deprovision":{"start":null,"end":null}}}`
statusAnnotation string = `{"operationalStatus":"OK","lastUpdated":"2020-04-15T15:00:50Z","hardwareProfile":"StatusProfile","hardware":{"systemVendor":{"manufacturer":"QEMU","productName":"Standard PC (Q35 + ICH9, 2009)","serialNumber":""},"firmware":{"bios":{"date":"","vendor":"","version":""}},"ramMebibytes":4096,"nics":[{"name":"eth0","model":"0x1af4 0x0001","mac":"00:b7:8b:bb:3d:f6","ip":"172.22.0.64","speedGbps":0,"vlanId":0,"pxe":true},{"name":"eth1","model":"0x1af4 0x0001","mac":"00:b7:8b:bb:3d:f8","ip":"192.168.111.20","speedGbps":0,"vlanId":0,"pxe":false}],"storage":[{"name":"/dev/sda","rotational":true,"sizeBytes":53687091200,"vendor":"QEMU","model":"QEMU HARDDISK","serialNumber":"drive-scsi0-0-0-0","hctl":"6:0:0:0"}],"cpu":{"arch":"x86_64","model":"Intel Xeon E3-12xx v2 (IvyBridge)","clockMegahertz":2494.224,"flags":["aes","apic","arat","avx","clflush","cmov","constant_tsc","cx16","cx8","de","eagerfpu","ept","erms","f16c","flexpriority","fpu","fsgsbase","fxsr","hypervisor","lahf_lm","lm","mca","mce","mmx","msr","mtrr","nopl","nx","pae","pat","pclmulqdq","pge","pni","popcnt","pse","pse36","rdrand","rdtscp","rep_good","sep","smep","sse","sse2","sse4_1","sse4_2","ssse3","syscall","tpr_shadow","tsc","tsc_adjust","tsc_deadline_timer","vme","vmx","vnmi","vpid","x2apic","xsave","xsaveopt","xtopology"],"count":4},"hostname":"node-0"},"provisioning":{"state":"provisioned","ID":"8a0ede17-7b87-44ac-9293-5b7d50b94b08","image":{"url":"bar","checksum":""}},"goodCredentials":{"credentials":{"name":"node-0-bmc-secret","namespace":"metal3"},"credentialsVersion":"879"},"triedCredentials":{"credentials":{"name":"node-0-bmc-secret","namespace":"metal3"},"credentialsVersion":"879"},"errorMessage":"","poweredOn":true,"operationHistory":{"register":{"start":"2020-04-15T12:06:26Z","end":"2020-04-15T12:07:12Z"},"inspect":{"start":"2020-04-15T12:07:12Z","end":"2020-04-15T12:09:29Z"},"provision":{"start":null,"end":null},"deprovision":{"start":null,"end":null}}}`
)

func init() {
Expand Down Expand Up @@ -200,7 +200,7 @@ func waitForProvisioningState(t *testing.T, r *ReconcileBareMetalHost, host *met
func TestStatusAnnotation_EmptyStatus(t *testing.T) {
host := newDefaultHost(t)
host.Annotations = map[string]string{
metal3v1alpha1.StatusAnnotation: statusAnnotaion,
metal3v1alpha1.StatusAnnotation: statusAnnotation,
}
host.Spec.Online = true
host.Spec.Image = &metal3v1alpha1.Image{URL: "foo", Checksum: "123"}
Expand All @@ -222,7 +222,7 @@ func TestStatusAnnotation_EmptyStatus(t *testing.T) {
func TestStatusAnnotation_StatusPresent(t *testing.T) {
host := newDefaultHost(t)
host.Annotations = map[string]string{
metal3v1alpha1.StatusAnnotation: statusAnnotaion,
metal3v1alpha1.StatusAnnotation: statusAnnotation,
}
host.Spec.Online = true
time := metav1.Now()
Expand All @@ -240,7 +240,44 @@ func TestStatusAnnotation_StatusPresent(t *testing.T) {
)
}

// TestStatusAnnotation tests if statusAnnotaion is populated correctly
// TestStatusAnnotation_Partial ensures that if the status annotation
// does not include the LastUpdated value reconciliation does not go
// into an infinite loop.
func TestStatusAnnotation_Partial(t *testing.T) {
// Build a version of the annotation text that does not include
// a LastUpdated value.
unpackedStatus, err := unmarshalStatusAnnotation([]byte(statusAnnotation))
if err != nil {
t.Fatal(err)
return
}
unpackedStatus.LastUpdated = nil
packedStatus, err := marshalStatusAnnotation(unpackedStatus)
if err != nil {
t.Fatal(err)
return
}

host := newDefaultHost(t)
host.Annotations = map[string]string{
metal3v1alpha1.StatusAnnotation: string(packedStatus),
}
host.Spec.Online = true
host.Spec.Image = &metal3v1alpha1.Image{URL: "foo", Checksum: "123"}

r := newTestReconciler(host)

tryReconcile(t, r, host,
func(host *metal3v1alpha1.BareMetalHost, result reconcile.Result) bool {
if host.Status.HardwareProfile == "StatusProfile" && host.Status.Provisioning.Image.URL == "bar" {
return true
}
return false
},
)
}

// TestStatusAnnotation tests if statusAnnotation is populated correctly
func TestStatusAnnotation(t *testing.T) {
host := newDefaultHost(t)
host.Spec.Online = true
Expand Down