Skip to content
This repository has been archived by the owner on Mar 31, 2023. It is now read-only.

Commit

Permalink
Fetch machine identity fewer times
Browse files Browse the repository at this point in the history
`resource.OS` was fetching `Name` and `Version` which were never used.
`os.OS.IDs()` was fetching UUIDs again, even though they had just been fetched.
Remove the code that implemented this, and a test that relied on it.
  • Loading branch information
bboreham committed Jul 10, 2020
1 parent 25e2a49 commit 85d95ee
Show file tree
Hide file tree
Showing 3 changed files with 2 additions and 82 deletions.
13 changes: 2 additions & 11 deletions pkg/apis/wksprovider/machine/os/os.go
Expand Up @@ -82,22 +82,13 @@ type Identifiers struct {
SystemUUID string
}

// IDs returns this machine's ID (see also OS#GetMachineID) and system UUID (see
// also: OS#GetSystemUUID).
// IDs returns this machine's ID system UUID.
func (o OS) IDs() (*Identifiers, error) {
osres, err := resource.NewOS(o.runner)
if err != nil {
return nil, err
}
machineID, err := osres.GetMachineID(o.runner)
if err != nil {
return nil, errors.Wrapf(err, "failed to read machine's ID")
}
systemUUID, err := osres.GetSystemUUID(o.runner)
if err != nil {
return nil, errors.Wrapf(err, "failed to read machine's system UUID")
}
return &Identifiers{MachineID: machineID, SystemUUID: systemUUID}, nil
return &Identifiers{MachineID: osres.MachineID, SystemUUID: osres.SystemUUID}, nil
}

type crdFile struct {
Expand Down
55 changes: 0 additions & 55 deletions pkg/plan/resource/os.go
Expand Up @@ -12,16 +12,6 @@ import (

// OS is a set of OS properties.
type OS struct {

// Name is the OS name, eg. 'centos' or 'debian'. On systemd OSes, this is the ID
// field of /etc/os-release. See:
// https://www.freedesktop.org/software/systemd/man/os-release.html
Name string `structs:"Name"`

// Version the OS version. On systemd OSes, this is the VERSION_ID field of
// /etc/os-release. See:
// https://www.freedesktop.org/software/systemd/man/os-release.html
Version string `structs:"Version"`
MachineID string `structs:"MachineID"`
SystemUUID string `structs:"SystemUUID"`

Expand Down Expand Up @@ -124,7 +114,6 @@ func (p *OS) State() plan.State {
}

var gatherFuncs []GatherFactFunc = []GatherFactFunc{
getOSRelease,
getMachineID,
getSystemUUID,
}
Expand Down Expand Up @@ -170,26 +159,6 @@ func (p *OS) Undo(r plan.Runner, current plan.State) error {
return nil
}

func getOSRelease(p *OS, r plan.Runner) error {
output, err := r.RunCommand("cat /etc/os-release", nil)
if err != nil {
return err
}
name := keyval(output, "ID")
if name == "" {
return fmt.Errorf("os: getOSRelease: could not query ID from output %s\n", output)
}
err = reflections.SetField(p, "Name", name)
if err != nil {
return err
}
version := keyval(output, "VERSION_ID")
if version == "" {
return errors.New("os: getOSRelease: could not query VERSION_ID")
}
return reflections.SetField(p, "Version", version)
}

func (p *OS) HasCommand(cmd string) (bool, error) {
// http://stackoverflow.com/questions/592620/how-to-check-if-a-program-exists-from-a-bash-script
_, err := p.runner.RunCommand(fmt.Sprintf("command -v -- %q >/dev/null 2>&1", cmd), nil)
Expand Down Expand Up @@ -253,34 +222,10 @@ func (p *OS) IsOSInContainerVM() (bool, error) {
return strings.Contains(output, "container=docker"), err
}

func (p *OS) GetMachineID(r plan.Runner) (string, error) {
err := getMachineID(p, r)
if err != nil {
return "", err
}
id, err := p.State().GetString("MachineID")
if err != nil {
return "", err
}
return id, nil
}

func getMachineID(p *OS, r plan.Runner) error {
return p.getValueFromFileContents(machineIDParams, r)
}

func (p *OS) GetSystemUUID(r plan.Runner) (string, error) {
err := getSystemUUID(p, r)
if err != nil {
return "", err
}
id, err := p.State().GetString("SystemUUID")
if err != nil {
return "", err
}
return id, nil
}

func getSystemUUID(p *OS, r plan.Runner) error {
return p.getValueFromFileContents(sysUuidParams, r)
}
Expand Down
16 changes: 0 additions & 16 deletions pkg/utilities/envcfg/envcfg_test.go
Expand Up @@ -258,22 +258,6 @@ func TestGetEnvSpecificConfig(t *testing.T) {
wantLockYUMPkgs: false,
wantNamespace: "foo",
},
{
name: "os-release error",
pkgType: resource.PkgTypeRPM,
runner: &fakeRunner{
value: map[string]runnerResult{
cmdOsRel: {out: relUbuntu, err: errors.New("kaboom")},
cmdEnv: {},
cmdSELinuxFound: {},
cmdMachineID: {out: "01234567"},
cmdUUID: {out: "01234567"},
cmdSELinuxPermissive: {},
cmdSELinuxEnforcing: {},
},
},
wantError: true,
},
{
name: "environ error",
pkgType: resource.PkgTypeRPM,
Expand Down

0 comments on commit 85d95ee

Please sign in to comment.