Run image build command like a phase

We perform almost the same activities prior calling createBootstrapIso
function like we do that in phase executor, also image build command
now is techically outdated since it doesn't work because it fails with
"phase document was not found" error. This patch removes unnecessary
functions like GenerateBootstrapIso and runs image build command in
a phase way. Test coverage increases slightly (by 0.2%) by removing
partially duplicating code which was not tested properly. Minor bug
fixes included.

Change-Id: I545004cea721164838b296ae647a7651cde248ff
Signed-off-by: Ruslan Aliev <raliev@mirantis.com>

cmd/image/build.go

@@ -17,19 +17,23 @@ package image
 import (
 	"github.com/spf13/cobra"
 
-	"opendev.org/airship/airshipctl/pkg/bootstrap/isogen"
 	"opendev.org/airship/airshipctl/pkg/config"
+	"opendev.org/airship/airshipctl/pkg/phase"
 )
 
 // NewImageBuildCommand creates a new command with the capability to build an ISO image.
 func NewImageBuildCommand(cfgFactory config.Factory) *cobra.Command {
 	var progress bool
-
 	cmd := &cobra.Command{
 		Use:   "build",
 		Short: "Build ISO image",
 		RunE: func(cmd *cobra.Command, args []string) error {
-			return isogen.GenerateBootstrapIso(cfgFactory, progress)
+			p := &phase.RunCommand{
+				Factory: cfgFactory,
+				Options: phase.RunFlags{Progress: progress},
+			}
+			p.Options.PhaseID.Name = config.BootstrapPhase
+			return p.RunE()
 		},
 	}
 

pkg/bootstrap/isogen/command.go

@@ -16,7 +16,6 @@ package isogen
 
 import (
 	"bufio"
-	"context"
 	"fmt"
 	"io"
 	"os"
@@ -65,69 +64,6 @@ type BootstrapIsoOptions struct {
 	writer   io.Writer
 }
 
-// GenerateBootstrapIso will generate data for cloud init and start ISO builder container
-// TODO (vkuzmin): Remove this public function and move another functions
-// to the executor module when the phases will be ready
-func GenerateBootstrapIso(cfgFactory config.Factory, progress bool) error {
-	ctx := context.Background()
-
-	globalConf, err := cfgFactory()
-	if err != nil {
-		return err
-	}
-
-	root, err := globalConf.CurrentContextEntryPoint(config.BootstrapPhase)
-	if err != nil {
-		return err
-	}
-	docBundle, err := document.NewBundleByPath(root)
-	if err != nil {
-		return err
-	}
-
-	imageConfiguration := &v1alpha1.ImageConfiguration{}
-	selector, err := document.NewSelector().ByObject(imageConfiguration, v1alpha1.Scheme)
-	if err != nil {
-		return err
-	}
-	doc, err := docBundle.SelectOne(selector)
-	if err != nil {
-		return err
-	}
-
-	err = doc.ToAPIObject(imageConfiguration, v1alpha1.Scheme)
-	if err != nil {
-		return err
-	}
-	if err = verifyInputs(imageConfiguration); err != nil {
-		return err
-	}
-
-	log.Print("Creating ISO builder container")
-	builder, err := container.NewContainer(
-		&ctx, imageConfiguration.Container.ContainerRuntime,
-		imageConfiguration.Container.Image)
-	if err != nil {
-		return err
-	}
-
-	bootstrapIsoOptions := BootstrapIsoOptions{
-		docBundle: docBundle,
-		builder:   builder,
-		doc:       doc,
-		cfg:       imageConfiguration,
-		debug:     log.DebugEnabled(),
-		progress:  progress,
-		writer:    log.Writer(),
-	}
-	err = bootstrapIsoOptions.createBootstrapIso()
-	if err != nil {
-		return err
-	}
-	log.Print("Checking artifacts")
-	return verifyArtifacts(imageConfiguration)
-}
-
 func verifyInputs(cfg *v1alpha1.ImageConfiguration) error {
 	if cfg.Container.Volume == "" {
 		return config.ErrMissingConfig{
@@ -227,7 +163,7 @@ func (opts BootstrapIsoOptions) createBootstrapIso() error {
 			case opts.debug:
 				log.Print("start reading container logs")
 				// either container log output or progress bar will be shown
-				if _, err = io.Copy(log.Writer(), cLogs); err != nil {
+				if _, err = io.Copy(opts.writer, cLogs); err != nil {
 					log.Debugf("failed to write container logs to log output %s", err)
 				}
 				log.Print("got EOF from container logs")

pkg/bootstrap/isogen/command_test.go

@@ -254,72 +254,6 @@ func TestVerifyInputs(t *testing.T) {
 	}
 }
 
-func TestGenerateBootstrapIso(t *testing.T) {
-	airshipConfigPath := "testdata/config/config"
-
-	t.Run("ContextEntryPointError", func(t *testing.T) {
-		cfg, err := config.CreateFactory(&airshipConfigPath)()
-		require.NoError(t, err)
-		cfg.Manifests["default"].Repositories = make(map[string]*config.Repository)
-		settings := func() (*config.Config, error) {
-			return cfg, nil
-		}
-		expectedErr := config.ErrMissingPhaseRepo{}
-		actualErr := GenerateBootstrapIso(settings, false)
-		assert.Equal(t, expectedErr, actualErr)
-	})
-
-	t.Run("NewBundleByPathError", func(t *testing.T) {
-		cfg, err := config.CreateFactory(&airshipConfigPath)()
-		require.NoError(t, err)
-		cfg.Manifests["default"].TargetPath = "/nonexistent"
-		settings := func() (*config.Config, error) {
-			return cfg, nil
-		}
-		expectedErr := config.ErrMissingPhaseDocument{PhaseName: "bootstrap"}
-		actualErr := GenerateBootstrapIso(settings, false)
-		assert.Equal(t, expectedErr, actualErr)
-	})
-
-	t.Run("SelectOneError", func(t *testing.T) {
-		cfg, err := config.CreateFactory(&airshipConfigPath)()
-		require.NoError(t, err)
-		cfg.Manifests["default"].SubPath = "missingkinddoc/site/test-site"
-		settings := func() (*config.Config, error) {
-			return cfg, nil
-		}
-		expectedErr := document.ErrDocNotFound{
-			Selector: document.NewSelector().ByGvk("airshipit.org", "v1alpha1", "ImageConfiguration")}
-		actualErr := GenerateBootstrapIso(settings, false)
-		assert.Equal(t, expectedErr, actualErr)
-	})
-
-	t.Run("ToObjectError", func(t *testing.T) {
-		cfg, err := config.CreateFactory(&airshipConfigPath)()
-		require.NoError(t, err)
-		cfg.Manifests["default"].SubPath = "missingmetadoc/site/test-site"
-		settings := func() (*config.Config, error) {
-			return cfg, nil
-		}
-		expectedErrMessage := "missing metadata.name in object"
-		actualErr := GenerateBootstrapIso(settings, false)
-		require.NotNil(t, actualErr)
-		assert.Contains(t, actualErr.Error(), expectedErrMessage)
-	})
-
-	t.Run("verifyInputsError", func(t *testing.T) {
-		cfg, err := config.CreateFactory(&airshipConfigPath)()
-		require.NoError(t, err)
-		cfg.Manifests["default"].SubPath = "missingvoldoc/site/test-site"
-		settings := func() (*config.Config, error) {
-			return cfg, nil
-		}
-		expectedErr := config.ErrMissingConfig{What: "Must specify volume bind for ISO builder container"}
-		actualErr := GenerateBootstrapIso(settings, false)
-		assert.Equal(t, expectedErr, actualErr)
-	})
-}
-
 func TestShowProgress(t *testing.T) {
 	tests := []struct {
 		name   string

pkg/bootstrap/isogen/executor.go

@@ -121,8 +121,8 @@ func (c *Executor) Run(evtCh chan events.Event, opts ifc.RunOptions) {
 		doc:       c.ExecutorDocument,
 		cfg:       c.imgConf,
 		debug:     log.DebugEnabled(),
-		progress:  false,
-		writer:    nil,
+		progress:  opts.Progress,
+		writer:    log.Writer(),
 	}
 
 	err := bootstrapOpts.createBootstrapIso()

pkg/bootstrap/isogen/testdata/config/config

@@ -1,34 +0,0 @@
-apiVersion: airshipit.org/v1alpha1
-clusters:
-  default:
-    clusterType:
-      ephemeral:
-        clusterKubeconf: default_ephemeral
-        managementConfiguration: default
-contexts:
-  default:
-    contextKubeconf: default_ephemeral
-    manifest: default
-currentContext: default
-kind: Config
-managementConfiguration:
-  default:
-    insecure: true
-    systemActionRetries: 30
-    systemRebootDelay: 30
-    type: redfish
-manifests:
-  default:
-    phaseRepositoryName: primary
-    repositories:
-      primary:
-        checkout:
-          branch: master
-          commitHash: ""
-          force: false
-          tag: ""
-        url: https://opendev.org/airship/treasuremap
-    subPath: primary/site/test-site
-    targetPath: testdata
-users:
-  admin: {}

pkg/bootstrap/isogen/testdata/config/kubeconfig

@@ -1,17 +0,0 @@
-apiVersion: v1
-clusters:
-- cluster:
-    server: https://172.17.0.1:6443
-  name: default_ephemeral
-contexts:
-- context:
-    cluster: default_ephemeral
-    user: admin
-  name: default
-current-context: default
-kind: Config
-preferences: {}
-users:
-- name: admin
-  user:
-    username: airship-admin

pkg/bootstrap/isogen/testdata/missingkinddoc/site/test-site/ephemeral/bootstrap/image_configuration.yaml

@@ -1,12 +0,0 @@
-apiVersion: airshipit.org/v1alpha1
-kind: FakeImageConfiguration
-metadata:
-  name: default
-builder:
-  networkConfigFileName: network-config
-  outputMetadataFileName: output-metadata.yaml
-  userDataFileName: user-data
-container:
-  containerRuntime: docker
-  image: quay.io/airshipit/isogen:latest-debian_stable
-  volume: /srv/iso:/config

pkg/bootstrap/isogen/testdata/missingkinddoc/site/test-site/ephemeral/bootstrap/kustomization.yaml

@@ -1,2 +0,0 @@
-resources:
-  - image_configuration.yaml

pkg/bootstrap/isogen/testdata/missingmetadoc/site/test-site/ephemeral/bootstrap/image_configuration.yaml

@@ -1,10 +0,0 @@
-apiVersion: airshipit.org/v1alpha1
-kind: ImageConfiguration
-builder:
-  networkConfigFileName: network-config
-  outputMetadataFileName: output-metadata.yaml
-  userDataFileName: user-data
-container:
-  containerRuntime: docker
-  image: quay.io/airshipit/isogen:latest-debian_stable
-  volume: /srv/iso:/config

pkg/bootstrap/isogen/testdata/missingmetadoc/site/test-site/ephemeral/bootstrap/kustomization.yaml

@@ -1,2 +0,0 @@
-resources:
-  - image_configuration.yaml

pkg/bootstrap/isogen/testdata/missingvoldoc/site/test-site/ephemeral/bootstrap/image_configuration.yaml

@@ -1,11 +0,0 @@
-apiVersion: airshipit.org/v1alpha1
-kind: ImageConfiguration
-metadata:
-  name: default
-builder:
-  networkConfigFileName: network-config
-  outputMetadataFileName: output-metadata.yaml
-  userDataFileName: user-data
-container:
-  containerRuntime: docker
-  image: quay.io/airshipit/isogen:latest-debian_stable

pkg/bootstrap/isogen/testdata/missingvoldoc/site/test-site/ephemeral/bootstrap/kustomization.yaml

@@ -1,2 +0,0 @@
-resources:
-  - image_configuration.yaml

pkg/phase/command.go

@@ -28,6 +28,7 @@ type RunFlags struct {
 	Timeout    time.Duration
 	PhaseID    ifc.ID
 	Kubeconfig string
+	Progress   bool
 }
 
 // RunCommand phase run command
@@ -55,7 +56,7 @@ func (c *RunCommand) RunE() error {
 	if err != nil {
 		return err
 	}
-	return phase.Run(ifc.RunOptions{DryRun: c.Options.DryRun, Timeout: c.Options.Timeout})
+	return phase.Run(ifc.RunOptions{DryRun: c.Options.DryRun, Timeout: c.Options.Timeout, Progress: c.Options.Progress})
 }
 
 // PlanCommand plan command

pkg/phase/ifc/executor.go

@@ -34,7 +34,8 @@ type Executor interface {
 
 // RunOptions holds options for run method
 type RunOptions struct {
-	DryRun bool
+	DryRun   bool
+	Progress bool
 
 	Timeout time.Duration
 }

testdata/k8s/kubeconfig.yaml

@@ -1,17 +0,0 @@
-apiVersion: v1
-clusters:
-- cluster:
-    server: https://172.17.0.1:6443
-  name: default_target
-contexts:
-- context:
-    cluster: default_target
-    user: admin
-  name: default
-current-context: ""
-kind: Config
-preferences: {}
-users:
-- name: admin
-  user:
-    username: airship-admin