discoverySuite.TestDiscoverServiceLocalHost: invalid series for wily and vivid

[Curtis Hovey]

As seen in http://reports.vapour.ws/releases/2810/job/run-unit-tests-wily-amd64/attempt/3
there was only one failure (for wily and go 1.4.2). This test also fails on vivid.

FAIL: discovery_test.go:176: discoverySuite.TestDiscoverServiceLocalHost

[LOG] 0:00.000 DEBUG juju.service discovered init system "systemd" from series "wily"
[LOG] 0:00.000 DEBUG juju.service discovered init system "systemd" from local host
discovery_test.go:195:
    c.Assert(err, jc.ErrorIsNil)
... value *errors.Err = &errors.Err{message:"", cause:(*errors.errorString)(0xc20802b7c0), previous:(*errors.Err)(0xc208102820), file:"github.com/juju/juju/service/discovery.go", line:35} ("failed to find juju data dir for service \"a-service\": invalid series \"\"")
... error stack:
 invalid series ""
 github.com/juju/juju/service/service.go:127: failed to find juju data dir for service "a-service"
 github.com/juju/juju/service/discovery.go:35:

OOPS: 34 passed, 1 FAILED
--- FAIL: TestPackage (0.13s

[ccoblis (claudiu-coblis)]

This test also fails on CentOS.

[James Tunnicliffe (dooferlad)]

Have had a dig. In TestDiscoverServiceLocalHost, after disableVersionDiscovery is called, service.DiscoverService(s.name, s.conf) is called. This is where the problem lies. DiscoverService calls getVersion, which returns a zeroed structure, as you may expect.

jujuVersion := getVersion()
here.Is(jujuVersion) // {Major:0, Minor:0, Tag:"", Patch:0, Build:0}, Series:"", Arch:"", OS:0
here.Is(initName) // systemd
here.Is(conf) // {Desc:"some service", Transient:false, AfterStopped:"", Env:map[string]string(nil), Limit:map[string]int(nil), Timeout:0, ExecStart:"/path/to/some-command", ExecStopPost:"", Logfile:"", ExtraScript:"", ServiceBinary:"", ServiceArgs:[]string(nil)}
here.Is(name) // a-service
service, err := newService(name, conf, initName, jujuVersion.Series)

This is because disableVersionDiscovery looks like this:
func (dt discoveryTest) disableVersionDiscovery(s *discoverySuite) {
 s.PatchVersion(version.Binary{
  OS: version.Unknown,
 })
}
So the entire version is zeroed out. I don't know if the intent was to only zero version.Current.OS. If you only patch the OS version the test passes, but it may not be testing correctly in that case.

The problem seems to be that in service/service.go we take the InitSystemSystemd branch, which then calls paths.DataDir, which is what causes the failure.

func newService(name string, conf common.Conf, initSystem, series string) (Service, error) {
 switch initSystem {
 case InitSystemWindows:
  svc, err := windows.NewService(name, conf)
  if err != nil {
   return nil, errors.Annotatef(err, "failed to wrap service %q", name)
  }
  return svc, nil
 case InitSystemUpstart:
  return upstart.NewService(name, conf), nil
 case InitSystemSystemd:
  dataDir, err := paths.DataDir(series)
  if err != nil {
   return nil, errors.Annotatef(err, "failed to find juju data dir for service %q", name)
  }

  svc, err := systemd.NewService(name, conf, dataDir)
  if err != nil {
   return nil, errors.Annotatef(err, "failed to wrap service %q", name)
  }
  return svc, nil
 default:
  return nil, errors.NotFoundf("init system %q", initSystem)
 }
}

Naturally, when we have set the series string to an empty value:

// DataDir returns a filesystem path to the folder used by juju to
// store tools, charms, locks, etc
func DataDir(series string) (string, error) {
 return osVal(series, dataDir)
}

... calls ...

// osVal will lookup the value of the key valname
// in the apropriate map, based on the series. This will
// help reduce boilerplate code
func osVal(series string, valname osVarType) (string, error) {
 os, err := version.GetOSFromSeries(series)
 if err != nil {
  return "", err
 }
 switch os {
 case version.Windows:
  return winVals[valname], nil
 default:
  return nixVals[valname], nil
 }
}

... calls ...

// GetOSFromSeries will return the operating system based
// on the series that is passed to it
func GetOSFromSeries(series string) (OSType, error) {
 if series == "" {
  return Unknown, errors.NotValidf("series %q", series)
 }
 if _, ok := ubuntuSeries[series]; ok {
  return Ubuntu, nil
 }
 if _, ok := centosSeries[series]; ok {
  return CentOS, nil
 }
 for _, val := range windowsVersions {
  if va...

[James Tunnicliffe (dooferlad)]

https://github.com/juju/juju/pull/2955 fixes this. It merged.