MGMT-9451: Moving to new service-agent protocol

go.mod

@@ -4,7 +4,6 @@ go 1.17
 
 require (
 	github.com/alecthomas/units v0.0.0-20190924025748-f65c72e2690d
-	github.com/alessio/shellescape v1.4.1
 	github.com/asaskevich/govalidator v0.0.0-20210307081110-f21760c49a8d
 	github.com/aws/aws-sdk-go v1.34.28
 	github.com/buger/jsonparser v1.1.1

go.sum

@@ -122,8 +122,6 @@ github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRF
 github.com/alecthomas/units v0.0.0-20190717042225-c3de453c63f4/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0=
 github.com/alecthomas/units v0.0.0-20190924025748-f65c72e2690d h1:UQZhZ2O0vMHr2cI+DC1Mbh0TJxzA3RcLoMsFw+aXw7E=
 github.com/alecthomas/units v0.0.0-20190924025748-f65c72e2690d/go.mod h1:rBZYJk541a8SKzHPHnH3zbiI+7dagKZ0cgpgrD7Fyho=
-github.com/alessio/shellescape v1.4.1 h1:V7yhSDDn8LP4lc4jS8pFkt0zCnzVJlG5JXy9BVKJUX0=
-github.com/alessio/shellescape v1.4.1/go.mod h1:PZAiSCk0LJaZkiCSkPv8qIobYglO3FPpyFjDCtHLS30=
 github.com/andreyvit/diff v0.0.0-20170406064948-c7f18ee00883/go.mod h1:rCTlJbsFo29Kk6CurOXKm700vrz8f0KW0JNfpkRJY/8=
 github.com/andybalholm/cascadia v1.1.0/go.mod h1:GsXiBklL0woXo1j/WYWtSYYC4ouU9PqHO0sqidkEA4Y=
 github.com/antihax/optional v1.0.0/go.mod h1:uupD/76wgC+ih3iEmQUL+0Ugr19nfwCT1kdvxnR2qWY=

internal/bminventory/inventory.go

@@ -2877,7 +2877,7 @@ func (b *bareMetalInventory) getImageInfo(clusterId *strfmt.UUID) (*models.Image
 	return &models.ImageInfo{}, nil
 }
 
-func (b *bareMetalInventory) generateV2NextStepRunnerCommand(ctx context.Context, params *installer.V2RegisterHostParams) *models.HostRegistrationResponseAO1NextStepRunnerCommand {
+func (b *bareMetalInventory) generateV2NextStepRunnerCommand(ctx context.Context, params *installer.V2RegisterHostParams) (*models.HostRegistrationResponseAO1NextStepRunnerCommand, error) {
 
 	if params.NewHostParams.DiscoveryAgentVersion != b.AgentDockerImg {
 		log := logutil.FromContext(ctx, b.log)
@@ -2887,17 +2887,17 @@ func (b *bareMetalInventory) generateV2NextStepRunnerCommand(ctx context.Context
 
 	config := hostcommands.NextStepRunnerConfig{
 		ServiceBaseURL:       b.ServiceBaseURL,
-		InfraEnvID:           params.InfraEnvID.String(),
-		HostID:               params.NewHostParams.HostID.String(),
+		InfraEnvID:           params.InfraEnvID,
+		HostID:               *params.NewHostParams.HostID,
 		UseCustomCACert:      b.ServiceCACertPath != "",
 		NextStepRunnerImage:  b.AgentDockerImg,
 		SkipCertVerification: b.SkipCertVerification,
 	}
-	command, args := hostcommands.GetNextStepRunnerCommand(&config)
+	command, args, err := hostcommands.GetNextStepRunnerCommand(&config)
 	return &models.HostRegistrationResponseAO1NextStepRunnerCommand{
 		Command: command,
 		Args:    *args,
-	}
+	}, err
 }
 
 func returnRegisterHostTransitionError(
@@ -4655,9 +4655,15 @@ func (b *bareMetalInventory) V2RegisterHost(ctx context.Context, params installe
 	eventgen.SendHostRegistrationSucceededEvent(ctx, b.eventsHandler, *params.NewHostParams.HostID,
 		params.InfraEnvID, host.ClusterID, hostutil.GetHostnameForMsg(host))
 
+	nextStepRunnerCommand, err := b.generateV2NextStepRunnerCommand(ctx, &params)
+	if err != nil {
+		log.WithError(err).Errorf("Fail to create nextStepRunnerCommand")
+		return common.GenerateErrorResponder(err)
+	}
+
 	hostRegistration := models.HostRegistrationResponse{
 		Host:                  *host,
-		NextStepRunnerCommand: b.generateV2NextStepRunnerCommand(ctx, &params),
+		NextStepRunnerCommand: nextStepRunnerCommand,
 	}
 
 	if err := tx.Commit().Error; err != nil {

internal/bminventory/inventory_test.go

@@ -455,7 +455,7 @@ var _ = Describe("RegisterHost", func() {
 				Expect(payload).ShouldNot(BeNil())
 				command := payload.NextStepRunnerCommand
 				Expect(command).ShouldNot(BeNil())
-				Expect(command.Command).ShouldNot(BeEmpty())
+				Expect(command.Command).Should(BeEmpty())
 				Expect(command.Args).ShouldNot(BeEmpty())
 			})
 		}
@@ -638,7 +638,7 @@ var _ = Describe("v2RegisterHost", func() {
 				Expect(payload).ShouldNot(BeNil())
 				command := payload.NextStepRunnerCommand
 				Expect(command).ShouldNot(BeNil())
-				Expect(command.Command).ShouldNot(BeEmpty())
+				Expect(command.Command).Should(BeEmpty())
 				Expect(command.Args).ShouldNot(BeEmpty())
 			})
 		}
@@ -10900,23 +10900,26 @@ var _ = Describe("update image version", func() {
 		agentImage := fmt.Sprintf("%s:%s", "quay.io/ocpmetal/agent", uuid.New().String())
 		bm.AgentDockerImg = agentImage
 		params.NewHostParams.DiscoveryAgentVersion = agentImage
-		bm.generateV2NextStepRunnerCommand(ctx, params)
+		_, err := bm.generateV2NextStepRunnerCommand(ctx, params)
+		Expect(err).NotTo(HaveOccurred())
 		Expect(logHook.AllEntries()).To(BeEmpty())
 	})
 
 	It("image tag mismatch", func() {
 		imageName := "quay.io/edge-infrastructure/assisted-installer-agent"
 		bm.AgentDockerImg = fmt.Sprintf("%s:%s", imageName, uuid.New().String())
 		params.NewHostParams.DiscoveryAgentVersion = fmt.Sprintf("%s:%s", imageName, uuid.New().String())
-		bm.generateV2NextStepRunnerCommand(ctx, params)
+		_, err := bm.generateV2NextStepRunnerCommand(ctx, params)
+		Expect(err).NotTo(HaveOccurred())
 		Expect(logHook.LastEntry().Message).To(ContainSubstring("uses an outdated agent image"))
 	})
 
 	It("image name mismatch", func() {
 		imageTag := uuid.New().String()
 		bm.AgentDockerImg = fmt.Sprintf("%s:%s", "quay.io/edge-infrastructure/assisted-installer-agent", imageTag)
 		params.NewHostParams.DiscoveryAgentVersion = fmt.Sprintf("%s:%s", "quay.io/ocpmetal/agent", imageTag)
-		bm.generateV2NextStepRunnerCommand(ctx, params)
+		_, err := bm.generateV2NextStepRunnerCommand(ctx, params)
+		Expect(err).NotTo(HaveOccurred())
 		Expect(logHook.LastEntry().Message).To(ContainSubstring("uses an outdated agent image"))
 	})
 
@@ -10925,7 +10928,8 @@ var _ = Describe("update image version", func() {
 		imageName := "edge-infrastructure/assisted-installer-agent"
 		bm.AgentDockerImg = fmt.Sprintf("%s/%s:%s", "quay.io", imageName, imageTag)
 		params.NewHostParams.DiscoveryAgentVersion = fmt.Sprintf("%s/%s:%s", "docker.io", imageName, imageTag)
-		bm.generateV2NextStepRunnerCommand(ctx, params)
+		_, err := bm.generateV2NextStepRunnerCommand(ctx, params)
+		Expect(err).NotTo(HaveOccurred())
 		Expect(logHook.LastEntry().Message).To(ContainSubstring("uses an outdated agent image"))
 	})
 })

internal/host/hostcommands/install_cmd.go

@@ -7,7 +7,6 @@ import (
 	"net"
 	"strings"
 
-	"github.com/alessio/shellescape"
 	"github.com/go-openapi/strfmt"
 	"github.com/go-openapi/swag"
 	"github.com/openshift/assisted-service/internal/common"
@@ -35,16 +34,6 @@ type installCmd struct {
 	versionsHandler   versions.Handler
 }
 
-var podmanBaseCmd = [...]string{
-	"podman", "run", "--privileged", "--pid=host", "--net=host", "--name=assisted-installer",
-	"--volume", "/dev:/dev:rw",
-	"--volume", "/opt:/opt:rw",
-	"--volume", "/var/log:/var/log:rw",
-	"--volume", "/run/systemd/journal/socket:/run/systemd/journal/socket",
-	"--volume", "/etc/pki:/etc/pki",
-	"--env=PULL_SECRET_TOKEN",
-}
-
 func NewInstallCmd(log logrus.FieldLogger, db *gorm.DB, hwValidator hardware.Validator, ocRelease oc.Release,
 	instructionConfig InstructionConfig, eventsHandler eventsapi.Handler, versionsHandler versions.Handler) *installCmd {
 	return &installCmd{
@@ -61,7 +50,6 @@ func NewInstallCmd(log logrus.FieldLogger, db *gorm.DB, hwValidator hardware.Val
 func (i *installCmd) GetSteps(ctx context.Context, host *models.Host) ([]*models.Step, error) {
 	step := &models.Step{}
 	step.StepType = models.StepTypeInstall
-	step.Command = "bash"
 
 	cluster, err := common.GetClusterFromDBWithHosts(i.db, *host.ClusterID)
 	if err != nil {
@@ -92,7 +80,7 @@ func (i *installCmd) GetSteps(ctx context.Context, host *models.Host) ([]*models
 		return nil, err
 	}
 
-	step.Args = []string{"-c", fullCmd}
+	step.Args = []string{fullCmd}
 
 	if _, err := hostutil.UpdateHost(i.log, i.db, host.InfraEnvID, *host.ID, *host.Status,
 		"installer_version", i.instructionConfig.InstallerImage); err != nil {
@@ -113,17 +101,19 @@ func (i *installCmd) getFullInstallerCommand(cluster *common.Cluster, host *mode
 		haMode = *cluster.HighAvailabilityMode
 	}
 
-	podmanCmd := podmanBaseCmd[:]
-	installerCmdArgs := []string{
-		"--role", string(role),
-		"--infra-env-id", host.InfraEnvID.String(),
-		"--cluster-id", host.ClusterID.String(),
-		"--host-id", string(*host.ID),
-		"--boot-device", bootdevice,
-		"--url", i.instructionConfig.ServiceBaseURL,
-		"--high-availability-mode", haMode,
-		"--controller-image", i.instructionConfig.ControllerImage,
-		"--agent-image", i.instructionConfig.AgentImage,
+	request := models.InstallCmdRequest{
+		Role:                 &role,
+		BaseURL:              swag.String(strings.TrimSpace(i.instructionConfig.ServiceBaseURL)),
+		ClusterID:            host.ClusterID,
+		HostID:               host.ID,
+		InfraEnvID:           &host.InfraEnvID,
+		Insecure:             swag.Bool(i.instructionConfig.SkipCertVerification),
+		Bootdevice:           swag.String(bootdevice),
+		HighAvailabilityMode: &haMode,
+		ControllerImage:      swag.String(i.instructionConfig.ControllerImage),
+		DisksToFormat:        disksToFormat,
+		CheckCvo:             swag.Bool(i.instructionConfig.CheckClusterVersion),
+		InstallerImage:       swag.String(i.instructionConfig.InstallerImage),
 	}
 
 	// those flags are not used on day2 installation
@@ -133,47 +123,26 @@ func (i *installCmd) getFullInstallerCommand(cluster *common.Cluster, host *mode
 			return "", err
 		}
 
-		mcoImage, err := i.ocRelease.GetMCOImage(i.log, *releaseImage.URL, i.instructionConfig.ReleaseImageMirror, cluster.PullSecret)
+		request.McoImage, err = i.ocRelease.GetMCOImage(i.log, *releaseImage.URL, i.instructionConfig.ReleaseImageMirror, cluster.PullSecret)
 		if err != nil {
 			return "", err
 		}
-		i.log.Infof("Install command releaseImage: %s, mcoImage: %s", *releaseImage.URL, mcoImage)
+		i.log.Infof("Install command releaseImage: %s, mcoImage: %s", *releaseImage.URL, request.McoImage)
 
 		mustGatherMap, err := i.versionsHandler.GetMustGatherImages(cluster.OpenshiftVersion, cluster.CPUArchitecture, cluster.PullSecret)
 		if err != nil {
 			return "", err
 		}
-		mustGatherImages, err := i.getMustGatherArgument(mustGatherMap)
+		request.MustGatherImage, err = i.getMustGatherArgument(mustGatherMap)
 		if err != nil {
 			return "", err
 		}
 
-		installerCmdArgs = append(installerCmdArgs, "--must-gather-image", mustGatherImages)
-		installerCmdArgs = append(installerCmdArgs, "--openshift-version", cluster.OpenshiftVersion)
-		installerCmdArgs = append(installerCmdArgs, "--mco-image", mcoImage)
-	}
-
-	for _, diskToFormat := range disksToFormat {
-		installerCmdArgs = append(installerCmdArgs, "--format-disk")
-		installerCmdArgs = append(installerCmdArgs, diskToFormat)
-	}
-
-	/*
-		boolean flag must be used either without value (flag present means True) or in the format of <flag>=True|False.
-		format <boolean flag> <value> is not supported by golang flag package and will cause the flags processing to finish
-		before processing the rest of the input flags
-	*/
-	if i.instructionConfig.SkipCertVerification {
-		installerCmdArgs = append(installerCmdArgs, "--insecure")
-	}
-
-	if i.instructionConfig.CheckClusterVersion {
-		installerCmdArgs = append(installerCmdArgs, "--check-cluster-version")
+		request.OpenshiftVersion = cluster.OpenshiftVersion
 	}
 
 	if i.hasCACert() {
-		podmanCmd = append(podmanCmd, "--volume", fmt.Sprintf("%s:%s:rw", common.HostCACertPath, common.HostCACertPath))
-		installerCmdArgs = append(installerCmdArgs, "--cacert", common.HostCACertPath)
+		request.CaCertPath = common.HostCACertPath
 	}
 
 	hostInstallerArgs, err := constructHostInstallerArgs(cluster, host, infraEnv, i.log)
@@ -182,57 +151,42 @@ func (i *installCmd) getFullInstallerCommand(cluster *common.Cluster, host *mode
 	}
 
 	if hostInstallerArgs != "" {
-		installerCmdArgs = append(installerCmdArgs, "--installer-args", hostInstallerArgs)
+		request.InstallerArgs = hostInstallerArgs
 	}
 
-	noProxyArgs := i.getProxyArguments(cluster.Name, cluster.BaseDNSDomain, cluster.HTTPProxy, cluster.HTTPSProxy, cluster.NoProxy)
-	if len(noProxyArgs) > 0 {
-		installerCmdArgs = append(installerCmdArgs, noProxyArgs...)
-	}
+	request.Proxy = i.getProxyArguments(cluster.Name, cluster.BaseDNSDomain, cluster.HTTPProxy, cluster.HTTPSProxy, cluster.NoProxy)
 
 	if i.instructionConfig.ServiceIPs != "" {
-		installerCmdArgs = append(installerCmdArgs, "--service-ips", i.instructionConfig.ServiceIPs)
-	}
-
-	return fmt.Sprintf("%s %s %s", shellescape.QuoteCommand(podmanCmd), i.instructionConfig.InstallerImage,
-		shellescape.QuoteCommand(installerCmdArgs)), nil
-}
-
-func (i *installCmd) getMustGatherArgument(mustGatherMap versions.MustGatherVersion) (string, error) {
-	//for backward compatability, if must gather images map contains only the ocp must gather
-	//we shall send a single image. otherwise, we shall send a json structure holding all the
-	//relevant images
-	if len(mustGatherMap) == 1 && mustGatherMap["ocp"] != "" {
-		return mustGatherMap["ocp"], nil
+		request.ServiceIps = strings.Split(i.instructionConfig.ServiceIPs, ",")
 	}
 
-	arg, err := json.Marshal(mustGatherMap)
+	b, err := json.Marshal(&request)
 	if err != nil {
-		i.log.WithError(err).Errorf("can not encode must-gather image map")
+		i.log.WithError(err).Warn("Json marshal")
 		return "", err
 	}
-	return string(arg), nil
+
+	return string(b), nil
 }
 
-func (i *installCmd) getProxyArguments(clusterName, baseDNSDomain, httpProxy, httpsProxy, noProxy string) []string {
-	cmd := make([]string, 0)
+func (i *installCmd) getProxyArguments(clusterName, baseDNSDomain, httpProxy, httpsProxy, noProxy string) *models.Proxy {
+	var proxy models.Proxy
 	if httpProxy == "" && httpsProxy == "" {
-		return cmd
+		return nil
 	}
 
 	if httpProxy != "" {
-		cmd = append(cmd, "--http-proxy", httpProxy)
+		proxy.HTTPProxy = swag.String(httpProxy)
 	}
 
 	if httpsProxy != "" {
-		cmd = append(cmd, "--https-proxy", httpsProxy)
+		proxy.HTTPSProxy = swag.String(httpsProxy)
 	}
 
 	noProxyTrim := strings.TrimSpace(noProxy)
 	if noProxyTrim == "*" {
-		cmd = append(cmd, "--no-proxy", noProxyTrim)
+		proxy.NoProxy = swag.String(noProxyTrim)
 	} else {
-
 		noProxyUpdated := []string{}
 		if noProxyTrim != "" {
 			noProxyUpdated = append(noProxyUpdated, noProxyTrim)
@@ -245,10 +199,26 @@ func (i *installCmd) getProxyArguments(clusterName, baseDNSDomain, httpProxy, ht
 			".svc",
 			".cluster.local",
 			fmt.Sprintf("api-int.%s.%s", clusterName, baseDNSDomain))
-		cmd = append(cmd, "--no-proxy", strings.Join(noProxyUpdated, ","))
+		proxy.NoProxy = swag.String(strings.Join(noProxyUpdated, ","))
+	}
+
+	return &proxy
+}
+
+func (i *installCmd) getMustGatherArgument(mustGatherMap versions.MustGatherVersion) (string, error) {
+	//for backward compatability, if must gather images map contains only the ocp must gather
+	//we shall send a single image. otherwise, we shall send a json structure holding all the
+	//relevant images
+	if len(mustGatherMap) == 1 && mustGatherMap["ocp"] != "" {
+		return mustGatherMap["ocp"], nil
 	}
 
-	return cmd
+	arg, err := json.Marshal(mustGatherMap)
+	if err != nil {
+		i.log.WithError(err).Errorf("can not encode must-gather image map")
+		return "", err
+	}
+	return string(arg), nil
 }
 
 func (i *installCmd) hasCACert() bool {