Skip to content

Commit

Permalink
Merge pull request juju#11149 from manadart/2.7-systemd-upgrades
Browse files Browse the repository at this point in the history 
juju#11149

### Checklist

 - [x] Checked if it requires a [pylibjuju](https://github.com/juju/python-libjuju) change?
 - [x] Added [integration tests](https://github.com/juju/juju/tree/develop/tests) for the PR?
 - [x] Added or updated [doc.go](https://discourse.jujucharms.com/t/readme-in-packages/451) related to packages changed?
 - [x] Do comments answer the question of why design decisions were made?

----

## Description of change

This patch is a follow-up to juju#11147 and precedes forthcoming work on accompanying upgrade steps.

It is refactoring only; removing an unused argument and adding various modularisations.

Accompanying mocks are regenerated.

## QA steps

No functional changes. Unit tests pass.

## Documentation changes

None.

## Bug reference

N/A
  • Loading branch information
@jujubot
jujubot authored Jan 24, 2020
2 parents 7a38d09 + de990d9 commit ba9f119
Show file tree
Hide file tree
Showing 9 changed files with 166 additions and 165 deletions.
133 changes: 73 additions & 60 deletions service/agentconf.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ type SystemdServiceManager interface {
// WriteSystemdAgents creates systemd files and create symlinks for the
// list of machine and units passed in the standard filepath.
WriteSystemdAgents(
machineAgent string, unitAgents []string, dataDir, symLinkSystemdDir, symLinkSystemdMultiUserDir string,
machineAgent string, unitAgents []string, dataDir, symLinkSystemdMultiUserDir string,
) ([]string, []string, []string, error)

//CreateAgentConf creates the configfile for specified agent running on a
Expand All @@ -53,13 +53,14 @@ type SystemdServiceManager interface {

// CopyAgentBinary copies all the tools into the path specified for each agent.
CopyAgentBinary(
machineAgent string, unitAgents []string, dataDir, toSeries, fromSeries string, jujuVersion version.Number) error
machineAgent string, unitAgents []string, dataDir, toSeries, fromSeries string, jujuVersion version.Number,
) error

// StartAllAgents starts all the agents in the machine with specified series.
StartAllAgents(machineAgent string, unitAgents []string, dataDir string) (string, []string, error)

// WriteServiceFiles writes the service files for machine and unit agents
// in the '/lib/systemd/system' path.
// in the /etc/systemd/system path.
WriteServiceFiles() error
}

Expand Down Expand Up @@ -92,7 +93,7 @@ func NewServiceManager(
}

// WriteServiceFiles writes service files to the standard
// "/lib/systemd/system" path.
// /etc/systemd/system path.
func (s *systemdServiceManager) WriteServiceFiles() error {
machineAgent, unitAgents, _, err := s.FindAgents(paths.NixDataDir)
if err != nil {
Expand All @@ -103,7 +104,6 @@ func (s *systemdServiceManager) WriteServiceFiles() error {
machineAgent,
unitAgents,
paths.NixDataDir,
systemd.EtcSystemdDir,
systemd.EtcSystemdMultiUserDir,
)
if err != nil {
Expand Down Expand Up @@ -136,7 +136,7 @@ func (s *systemdServiceManager) FindAgents(dataDir string) (string, []string, []
if err != nil {
return "", nil, nil, errors.Annotate(err, "opening agents dir")
}
defer dir.Close()
defer func() { _ = dir.Close() }()

entries, err := dir.Readdir(-1)
if err != nil {
Expand Down Expand Up @@ -164,79 +164,92 @@ func (s *systemdServiceManager) FindAgents(dataDir string) (string, []string, []
// WriteSystemdAgents creates systemd files and symlinks for the input machine
// and unit agents, in the standard filepath '/var/lib/juju'.
func (s *systemdServiceManager) WriteSystemdAgents(
machineAgent string, unitAgents []string, dataDir, symLinkSystemdDir, symLinkSystemdMultiUserDir string,
machineAgent string, unitAgents []string, dataDir, systemdMultiUserDir string,
) ([]string, []string, []string, error) {
var (
startedSysServiceNames []string
startedSymServiceNames []string
errAgentNames []string
lastError error
autoLinkedServiceNames []string
manualLinkedServiceNames []string
errAgentNames []string
lastError error
)

for _, agentName := range append(unitAgents, machineAgent) {
conf, err := s.CreateAgentConf(agentName, dataDir)
systemdLinked, err := s.writeSystemdAgent(agentName, dataDir, systemdMultiUserDir)
if err != nil {
logger.Infof("%s", err)
logger.Errorf("service creation failed for %s: %s", agentName, err.Error())
errAgentNames = append(errAgentNames, agentName)
lastError = err
continue
}

svcName := serviceName(agentName)
svc, err := s.newService(svcName, conf)
if err != nil {
logger.Infof("Failed to create new service %s: ", err)
if systemdLinked {
autoLinkedServiceNames = append(autoLinkedServiceNames, serviceName(agentName))
continue
}
manualLinkedServiceNames = append(manualLinkedServiceNames, serviceName(agentName))
}
return autoLinkedServiceNames, manualLinkedServiceNames, errAgentNames, lastError
}

uSvc, ok := svc.(UpgradableService)
if !ok {
return nil, nil, nil, errors.Errorf("%s service not of type UpgradableService", svcName)
}
// WriteSystemdAgents creates systemd files and symlinks for the input
// agentName.
// The boolean return indicates whether systemd automatically linked the file
// into the multi-user-target directory.
func (s *systemdServiceManager) writeSystemdAgent(agentName, dataDir, systemdMultiUserDir string) (bool, error) {
conf, err := s.CreateAgentConf(agentName, dataDir)
if err != nil {
return false, errors.Trace(err)
}

dbusMethodFound := true
if err = uSvc.WriteService(); err != nil {
// Note that this error is already logged by the systemd package.

// This is not ideal, but it is possible on an Upstart-based OS
// (such as Trusty) for run/systemd/system to exist, which is used
// for detection of systemd as the running init system.
// If this happens, then D-Bus will error with the message below.
// We need to detect this condition and fall through to linking the
// service files manually.
if strings.Contains(strings.ToLower(err.Error()), "no such method") {
dbusMethodFound = false
logger.Infof("attempting to manually link service file for %s", agentName)
} else {
errAgentNames = append(errAgentNames, agentName)
lastError = err
continue
}
} else {
logger.Infof("successfully wrote service for %s:", agentName)
}
svcName := serviceName(agentName)
svc, err := s.newService(svcName, conf)
if err != nil {
return false, errors.Annotate(err, "creating new service")
}

// If systemd is the running init system on this host, *and* if the
// call to DBusAPI.LinkUnitFiles in WriteService above returned no
// error, it will have resulted in updated sym-links for the file.
// We are done.
if s.isRunning() && dbusMethodFound {
startedSysServiceNames = append(startedSysServiceNames, svcName)
logger.Infof("wrote %s agent, enabled and linked by systemd", svcName)
continue
}
uSvc, ok := svc.(UpgradableService)
if !ok {
return false, errors.New("service not of type UpgradableService")
}

// Otherwise we need to manually ensure the service unit links.
svcFileName := svcName + ".service"
if err = os.Symlink(path.Join(systemd.EtcSystemdDir, svcFileName),
path.Join(symLinkSystemdMultiUserDir, svcFileName)); err != nil && !os.IsExist(err) {
return nil, nil, nil, errors.Errorf(
"failed to link service file (%s) in multi-user.target.wants dir: %s\n", svcFileName, err)
dbusMethodFound := true
if err = uSvc.WriteService(); err != nil {
// Note that this error is already logged by the systemd package.

// This is not ideal, but it is possible on an Upstart-based OS
// (such as Trusty) for run/systemd/system to exist, which is used
// for detection of systemd as the running init system.
// If this happens, then D-Bus will error with the message below.
// We need to detect this condition and fall through to linking the
// service files manually.
if !strings.Contains(strings.ToLower(err.Error()), "no such method") {
return false, errors.Trace(err)
} else {
dbusMethodFound = false
logger.Infof("attempting to manually link service file for %s", agentName)
}
} else {
logger.Infof("successfully wrote service for %s:", agentName)
}

// If systemd is the running init system on this host, *and* if the
// call to DBusAPI.LinkUnitFiles in WriteService above returned no
// error, it will have resulted in updated sym-links for the file.
// We are done.
if s.isRunning() && dbusMethodFound {
logger.Infof("wrote %s agent, enabled and linked by systemd", svcName)
return true, nil
}

startedSymServiceNames = append(startedSymServiceNames, svcName)
logger.Infof("wrote %s agent, enabled and linked by symlink", svcName)
// Otherwise we need to manually ensure the service unit links.
svcFileName := svcName + ".service"
if err = os.Symlink(path.Join(systemd.EtcSystemdDir, svcFileName),
path.Join(systemdMultiUserDir, svcFileName)); err != nil && !os.IsExist(err) {
return false, errors.Annotatef(err, "linking service file (%s) in multi-user.target.wants dir", svcFileName)
}
return startedSysServiceNames, startedSymServiceNames, errAgentNames, lastError

logger.Infof("wrote %s agent, enabled and linked by symlink", svcName)
return false, nil
}

// CreateAgentConf creates the configfile for specified agent running on a host with specified series.
Expand Down
10 changes: 5 additions & 5 deletions service/agentconf_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ func (s *agentConfSuite) listServices() ([]string, error) {
return s.serviceData.InstalledNames(), nil
}

func (s *agentConfSuite) newService(name string, conf common.Conf) (service.Service, error) {
func (s *agentConfSuite) newService(name string, _ common.Conf) (service.Service, error) {
for _, svc := range s.services {
if svc.Name() == name {
return svc, nil
Expand Down Expand Up @@ -290,7 +290,7 @@ func (s *agentConfSuite) TestCopyAgentBinaryOriginalAgentBinariesNotFound(c *gc.

func (s *agentConfSuite) TestWriteSystemdAgents(c *gc.C) {
startedSymLinkAgents, startedSysServiceNames, errAgents, err := s.manager.WriteSystemdAgents(
s.machineName, s.unitNames, s.systemdDataDir, s.systemdDir, s.systemdMultiUserDir)
s.machineName, s.unitNames, s.systemdDataDir, s.systemdMultiUserDir)

c.Assert(err, jc.ErrorIsNil)
c.Assert(startedSysServiceNames, gc.HasLen, 0)
Expand All @@ -306,7 +306,7 @@ func (s *agentConfSuite) TestWriteSystemdAgentsSystemdNotRunning(c *gc.C) {
)

startedSymLinkAgents, startedSysServiceNames, errAgents, err := s.manager.WriteSystemdAgents(
s.machineName, s.unitNames, s.systemdDataDir, s.systemdDir, s.systemdMultiUserDir)
s.machineName, s.unitNames, s.systemdDataDir, s.systemdMultiUserDir)

c.Assert(err, jc.ErrorIsNil)
c.Assert(startedSymLinkAgents, gc.HasLen, 0)
Expand All @@ -323,7 +323,7 @@ func (s *agentConfSuite) TestWriteSystemdAgentsDBusErrManualLink(c *gc.C) {
)

startedSymLinkAgents, startedSysServiceNames, errAgents, err := s.manager.WriteSystemdAgents(
s.machineName, s.unitNames, s.systemdDataDir, s.systemdDir, s.systemdMultiUserDir)
s.machineName, s.unitNames, s.systemdDataDir, s.systemdMultiUserDir)

c.Assert(err, jc.ErrorIsNil)

Expand All @@ -342,7 +342,7 @@ func (s *agentConfSuite) TestWriteSystemdAgentsWriteServiceFail(c *gc.C) {
)

startedSymLinkAgents, startedSysServiceNames, errAgents, err := s.manager.WriteSystemdAgents(
s.machineName, s.unitNames, s.systemdDataDir, s.systemdDir, s.systemdMultiUserDir)
s.machineName, s.unitNames, s.systemdDataDir, s.systemdMultiUserDir)

c.Assert(err, gc.ErrorMatches, "fail me")
c.Assert(startedSysServiceNames, gc.HasLen, 0)
Expand Down
48 changes: 24 additions & 24 deletions upgrades/steps_24.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,38 +65,38 @@ func stepsFor24() []Step {
&upgradeStep{
description: "Install the service file in Standard location '/lib/systemd'",
targets: []Target{AllMachines},
run: installServiceFile,
run: writeServiceFiles(true),
},
}
}

// install the service files in Standard location - '/lib/systemd/system path.
func installServiceFile(context Context) error {
hostSeries, err := series.HostSeries()
if err == nil {
// writeServiceFiles writes service files into the default systemd search path.
// The supplied boolean indicates whether the old
// /var/lib/init files should be removed.
func writeServiceFiles(cleanupOld bool) func(Context) error {
return func(ctx Context) error {
hostSeries, err := series.HostSeries()
if err != nil {
return errors.Trace(err)
}

initName, err := service.VersionInitSystem(hostSeries)
if err != nil {
logger.Errorf("unsuccessful writing the service files in /lib/systemd/system path")
return err
} else {
if initName == service.InitSystemSystemd {
oldDataDir := context.AgentConfig().DataDir()
oldInitDataDir := filepath.Join(oldDataDir, "init")
return errors.Annotate(err, "writing systemd service files")
}

sysdManager := service.NewServiceManagerWithDefaults()
err = sysdManager.WriteServiceFiles()
if err != nil {
logger.Errorf("unsuccessful writing the service files in /lib/systemd/system path")
return err
}
// Cleanup the old dir - /var/lib/init
return os.RemoveAll(oldInitDataDir)
} else {
logger.Infof("upgrade to systemd possible only for 'xenial' and above")
return nil
if initName == service.InitSystemSystemd {
if err := service.NewServiceManagerWithDefaults().WriteServiceFiles(); err != nil {
return errors.Annotate(err, "writing systemd service files")
}

if cleanupOld {
return errors.Trace(os.RemoveAll(filepath.Join(ctx.AgentConfig().DataDir(), "init")))
}
return nil
}
} else {
return errors.Trace(err)

logger.Infof("skipping upgrade for non systemd series %s", hostSeries)
return nil
}
}
32 changes: 1 addition & 31 deletions upgrades/steps_245.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,43 +3,13 @@

package upgrades

import (
"github.com/juju/utils/series"

"github.com/juju/juju/service"
)

// stepsFor245 returns upgrade steps for Juju 2.4.5
func stepsFor245() []Step {
return []Step{
&upgradeStep{
description: "update exec.start.sh log path if incorrect",
targets: []Target{AllMachines},
run: correctServiceFileLogPath,
run: writeServiceFiles(false),
},
}
}

// install the service files in Standard location - '/lib/systemd/system path.
func correctServiceFileLogPath(context Context) error {
hostSeries, err := series.HostSeries()
if err != nil {
logger.Errorf("getting host series: %e", err)
}
initName, err := service.VersionInitSystem(hostSeries)
if err != nil {
logger.Errorf("unsuccessful checking init script for correct log path: %e", err)
return err
}
if initName != service.InitSystemSystemd {
return nil
}
// rewrite files to correct errors in previous upgrade step
sysdManager := service.NewServiceManagerWithDefaults()
err = sysdManager.WriteServiceFiles()
if err != nil {
logger.Errorf("rewriting service file: %e", err)
return err
}
return nil
}
Loading

0 comments on commit ba9f119

Please sign in to comment.