Merge pull request juju#12859 from gabriel-samfira/fix-windows-userdata

juju#12859 This fixes an embarrassing bug of my own making, introduced in commit: 942456e where I omitted to remove the corresponding parameters to ```fmt.Sprintf``` after modifying the string it was formatting. I added a test for ```InstallCommands()``` as well as fixing some tests for password change that seem to have not been caught by the CI. Update: Given that unit agents seem to be gone now, and there is only one service being created on a machine, the machine agent is now run under the local jujud user on windows again. A followup PR will be created at a later date that will remove code that was used to maintain unit agents on Windows. ## QA steps ```bash juju bootstrap juju add-machine --series win2019 ``` Wait for machine to boot. ```bash juju run --machine 0 "sc.exe qc jujud-machine-0" ``` Check that ```SERVICE_START_NAME``` is ```.\jujud```.
sdg0919 · May 20, 2021 · e1cb593 · e1cb593
2 parents 4d120ac + abee5b2
commit e1cb593
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 11 deletions.
diff --git a/cloudconfig/windows_userdata_test.go b/cloudconfig/windows_userdata_test.go
@@ -764,7 +764,7 @@ mongoversion: "0.0"
 
 "@
 cmd.exe /C mklink /D C:\Juju\lib\juju\tools\machine-10 1.2.3-windows-amd64
-New-Service -Name 'jujud-machine-10' -DependsOn Winmgmt -DisplayName 'juju agent for machine-10' '"C:\Juju\lib\juju\tools\machine-10\jujud.exe" machine --data-dir "C:\Juju\lib\juju" --machine-id 10 --debug'
+New-Service -Credential $jujuCreds -Name 'jujud-machine-10' -DependsOn Winmgmt -DisplayName 'juju agent for machine-10' '"C:\Juju\lib\juju\tools\machine-10\jujud.exe" machine --data-dir "C:\Juju\lib\juju" --machine-id 10 --debug'
 sc.exe failure 'jujud-machine-10' reset=5 actions=restart/1000
-sc.exe failureflag 'juju agent for machine-10' 1%!(EXTRA string='"C:\Juju\lib\juju\tools\machine-10\jujud.exe" machine --data-dir "C:\Juju\lib\juju" --machine-id 10 --debug', string='jujud-machine-10', string='jujud-machine-10')
+sc.exe failureflag 'jujud-machine-10' 1
 Start-Service 'jujud-machine-10'`
diff --git a/service/windows/password_windows_test.go b/service/windows/password_windows_test.go
@@ -29,7 +29,7 @@ type myAmazingServiceManager struct {
 func (mgr *myAmazingServiceManager) ChangeServicePassword(name, newPassword string) error {
 	mgr.svcNames = append(mgr.svcNames, name)
 	mgr.pwd = newPassword
-	if name == "failme" {
+	if name == "jujud-unit-failme" {
 		return errors.New("wubwub")
 	}
 	return nil
@@ -48,11 +48,12 @@ func (s *ServicePasswordChangerSuite) SetUpTest(c *gc.C) {
 }
 
 func listServices() ([]string, error) {
-	return []string{"boom", "pow"}, nil
+	// all services except those prefixed with jujud-unit will be filtered out.
+	return []string{"boom", "pow", "jujud-unit-valid", "jujud-unit-another"}, nil
 }
 
 func listServicesFailingService() ([]string, error) {
-	return []string{"boom", "failme", "pow"}, nil
+	return []string{"boom", "pow", "jujud-unit-failme"}, nil
 }
 
 func brokenListServices() ([]string, error) {
@@ -62,7 +63,7 @@ func brokenListServices() ([]string, error) {
 func (s *ServicePasswordChangerSuite) TestChangeServicePasswordListSucceeds(c *gc.C) {
 	err := s.c.ChangeJujudServicesPassword("newPass", s.mgr, listServices)
 	c.Assert(err, jc.ErrorIsNil)
-	c.Assert(s.mgr.svcNames, gc.DeepEquals, []string{"boom", "pow"})
+	c.Assert(s.mgr.svcNames, gc.DeepEquals, []string{"jujud-unit-valid", "jujud-unit-another"})
 	c.Assert(s.mgr.pwd, gc.Equals, "newPass")
 }
 
@@ -74,7 +75,7 @@ func (s *ServicePasswordChangerSuite) TestChangeServicePasswordListFails(c *gc.C
 func (s *ServicePasswordChangerSuite) TestChangePasswordFails(c *gc.C) {
 	err := s.c.ChangeJujudServicesPassword("newPass", s.mgr, listServicesFailingService)
 	c.Assert(err, gc.ErrorMatches, "wubwub")
-	c.Assert(s.mgr.svcNames, gc.DeepEquals, []string{"boom", "failme"})
+	c.Assert(s.mgr.svcNames, gc.DeepEquals, []string{"jujud-unit-failme"})
 	c.Assert(s.mgr.pwd, gc.Equals, "newPass")
 }
 

diff --git a/service/windows/service.go b/service/windows/service.go
@@ -230,9 +230,6 @@ func (s *Service) Install() error {
 // InstallCommands returns shell commands to install the service.
 func (s *Service) InstallCommands() ([]string, error) {
 	cmd := fmt.Sprintf(serviceCreateCommandTemplate[1:],
-		renderer.Quote(s.Service.Name),
-		renderer.Quote(s.Service.Conf.Desc),
-		renderer.Quote(s.Service.Conf.ExecStart),
 		renderer.Quote(s.Service.Name),
 		renderer.Quote(s.Service.Conf.Desc),
 		renderer.Quote(s.Service.Conf.ExecStart),
@@ -249,6 +246,6 @@ func (s *Service) StartCommands() ([]string, error) {
 }
 
 const serviceCreateCommandTemplate = `
-New-Service -Name %s -DependsOn Winmgmt -DisplayName %s %s
+New-Service -Credential $jujuCreds -Name %s -DependsOn Winmgmt -DisplayName %s %s
 sc.exe failure %s reset=5 actions=restart/1000
 sc.exe failureflag %s 1`
diff --git a/service/windows/service_test.go b/service/windows/service_test.go
@@ -5,6 +5,8 @@
 package windows_test
 
 import (
+	"fmt"
+
 	"github.com/juju/errors"
 	"github.com/juju/testing"
 	jc "github.com/juju/testing/checkers"
@@ -206,3 +208,16 @@ func (s *serviceSuite) TestInstalledListError(c *gc.C) {
 	c.Assert(err.Error(), gc.Equals, listErr.Error())
 	c.Assert(exists, jc.IsFalse)
 }
+
+func (s *serviceSuite) TestInstallCommands(c *gc.C) {
+	commands, err := s.mgr.InstallCommands()
+	c.Assert(err, gc.IsNil)
+
+	expected := []string{
+		fmt.Sprintf("New-Service -Credential $jujuCreds -Name '%s' -DependsOn Winmgmt -DisplayName '%s' '%s'", s.name, s.conf.Desc, s.conf.ExecStart),
+		fmt.Sprintf("sc.exe failure '%s' reset=5 actions=restart/1000", s.name),
+		fmt.Sprintf("sc.exe failureflag '%s' 1", s.name),
+	}
+
+	c.Assert(commands, gc.DeepEquals, expected)
+}