canonical / gomaasclient

Go MAAS client
Apache License 2.0
23 stars 28 forks source link

Cannot skip commissioning when manually adding machine #75

Open gnuoy opened 6 months ago

gnuoy commented 6 months ago

<tl;dr> If omitempty is used with a boolean option then go-querystring drops it if it is set to false. Which leads to: "commissioning = false" => commissioning option dropped => maas runs commissioning </tl;dr>

When adding a machine manually via the CLI it is possible to skip commissioning by setting

maas admin machines create ... commission=false

This is not possible via the gomaasclient due to the way MachineParams are defined ( https://github.com/maas/gomaasclient/blob/master/entity/machine.go#L223 ) .

If commission is not explicitly set in a request to the maas api then maas does the default which is to run a commission. Which leads to the following:

If commission is unset:

    _, err = c.Machines.Create(
        &entity.MachineParams{
            Hostname:      "test01",
                        ...
        }

Maas will run a commission as this is the default behaviour

If commission is set to True:

    _, err = c.Machines.Create(
        &entity.MachineParams{
            Hostname:      "test01",
                        Commission:    true,
                        ...
        }

Maas will run a commission as explicitly requested

If commission is set to False:

    _, err = c.Machines.Create(
        &entity.MachineParams{
            Hostname:      "test01",
                        Commission:    false,
                        ...
        }

Maas will run a commission because when the machine params are processes the commission option is dropped (https://github.com/maas/gomaasclient/blob/master/client/machines.go#L37) because it is set to false and that option has omitempty set. omitempty omits the value if "...values are false, 0, any nil pointer or interface value, any array slice, map, or string of length zero, and any type (such as time.Time) that returns true for IsZero()."

gnuoy commented 6 months ago

A workaround is to disable automatic commissioning globally with:

maas admin maas set-config name=enlist_commissioning value=false

troyanov commented 6 months ago

Hi @gnuoy

Indeed, this is a valid bug.

import (
    "fmt"
    "github.com/google/go-querystring/query"
)

type Options struct {
        // Faulty behaviour with `omitempty`
    //Commission bool `url:"commission,omitempty"`
    Commission bool `url:"commission"`
}

func main() {
    opt := Options{false}
    v, _ := query.Values(opt)
        // Will print nothing instead of `commission=false` if `omitempty` is specified
    fmt.Print(v.Encode())
}
gnuoy commented 6 months ago

Thanks for the quick triage @troyanov ! fwiw I think the issue is going to affect any setting that:

  1. Is a bool
  2. Uses omitempty (I think all of them do at the moment)
  3. maas interprets the absence of the setting as true
kmullin commented 6 days ago

FWIW, I am also hitting this bool omitempty bug with NodeScripts.

My workflow:

The current workaround is to Delete the script and Create it again, but this wrecks havoc on any inflight tests to machines that may be running.