commit a6363f1c1e65bbeb43c0abf21e2be15d6cc019a1 Author: Ruslan Aliev Date: Wed Oct 7 22:58:00 2020 -0500 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 diff --git a/cmd/image/build.go b/cmd/image/build.go index f6ecd68..dff470d 100644 --- a/cmd/image/build.go +++ b/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() }, } diff --git a/pkg/bootstrap/isogen/command.go b/pkg/bootstrap/isogen/command.go index 3fb341b..bca6449 100644 --- a/pkg/bootstrap/isogen/command.go +++ b/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") diff --git a/pkg/bootstrap/isogen/command_test.go b/pkg/bootstrap/isogen/command_test.go index a3ff53b..5b7a9c6 100644 --- a/pkg/bootstrap/isogen/command_test.go +++ b/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 diff --git a/pkg/bootstrap/isogen/executor.go b/pkg/bootstrap/isogen/executor.go index ead0e90..9427bd8 100644 --- a/pkg/bootstrap/isogen/executor.go +++ b/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() diff --git a/pkg/bootstrap/isogen/testdata/config/config b/pkg/bootstrap/isogen/testdata/config/config deleted file mode 100644 index 2046db6..0000000 --- a/pkg/bootstrap/isogen/testdata/config/config +++ /dev/null @@ -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: {} diff --git a/pkg/bootstrap/isogen/testdata/config/kubeconfig b/pkg/bootstrap/isogen/testdata/config/kubeconfig deleted file mode 100644 index 0fc1297..0000000 --- a/pkg/bootstrap/isogen/testdata/config/kubeconfig +++ /dev/null @@ -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 diff --git a/pkg/bootstrap/isogen/testdata/missingkinddoc/site/test-site/ephemeral/bootstrap/image_configuration.yaml b/pkg/bootstrap/isogen/testdata/missingkinddoc/site/test-site/ephemeral/bootstrap/image_configuration.yaml deleted file mode 100644 index aecfb71..0000000 --- a/pkg/bootstrap/isogen/testdata/missingkinddoc/site/test-site/ephemeral/bootstrap/image_configuration.yaml +++ /dev/null @@ -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 diff --git a/pkg/bootstrap/isogen/testdata/missingkinddoc/site/test-site/ephemeral/bootstrap/kustomization.yaml b/pkg/bootstrap/isogen/testdata/missingkinddoc/site/test-site/ephemeral/bootstrap/kustomization.yaml deleted file mode 100644 index 86ef7db..0000000 --- a/pkg/bootstrap/isogen/testdata/missingkinddoc/site/test-site/ephemeral/bootstrap/kustomization.yaml +++ /dev/null @@ -1,2 +0,0 @@ -resources: - - image_configuration.yaml diff --git a/pkg/bootstrap/isogen/testdata/missingmetadoc/site/test-site/ephemeral/bootstrap/image_configuration.yaml b/pkg/bootstrap/isogen/testdata/missingmetadoc/site/test-site/ephemeral/bootstrap/image_configuration.yaml deleted file mode 100644 index 91e86a5..0000000 --- a/pkg/bootstrap/isogen/testdata/missingmetadoc/site/test-site/ephemeral/bootstrap/image_configuration.yaml +++ /dev/null @@ -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 diff --git a/pkg/bootstrap/isogen/testdata/missingmetadoc/site/test-site/ephemeral/bootstrap/kustomization.yaml b/pkg/bootstrap/isogen/testdata/missingmetadoc/site/test-site/ephemeral/bootstrap/kustomization.yaml deleted file mode 100644 index 86ef7db..0000000 --- a/pkg/bootstrap/isogen/testdata/missingmetadoc/site/test-site/ephemeral/bootstrap/kustomization.yaml +++ /dev/null @@ -1,2 +0,0 @@ -resources: - - image_configuration.yaml diff --git a/pkg/bootstrap/isogen/testdata/missingvoldoc/site/test-site/ephemeral/bootstrap/image_configuration.yaml b/pkg/bootstrap/isogen/testdata/missingvoldoc/site/test-site/ephemeral/bootstrap/image_configuration.yaml deleted file mode 100644 index cf65e1c..0000000 --- a/pkg/bootstrap/isogen/testdata/missingvoldoc/site/test-site/ephemeral/bootstrap/image_configuration.yaml +++ /dev/null @@ -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 diff --git a/pkg/bootstrap/isogen/testdata/missingvoldoc/site/test-site/ephemeral/bootstrap/kustomization.yaml b/pkg/bootstrap/isogen/testdata/missingvoldoc/site/test-site/ephemeral/bootstrap/kustomization.yaml deleted file mode 100644 index 86ef7db..0000000 --- a/pkg/bootstrap/isogen/testdata/missingvoldoc/site/test-site/ephemeral/bootstrap/kustomization.yaml +++ /dev/null @@ -1,2 +0,0 @@ -resources: - - image_configuration.yaml diff --git a/pkg/phase/command.go b/pkg/phase/command.go index 17f9236..a2c915c 100644 --- a/pkg/phase/command.go +++ b/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 diff --git a/pkg/phase/ifc/executor.go b/pkg/phase/ifc/executor.go index a1f6f8f..c7da6a4 100644 --- a/pkg/phase/ifc/executor.go +++ b/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 } diff --git a/testdata/k8s/kubeconfig.yaml b/testdata/k8s/kubeconfig.yaml deleted file mode 100644 index 43c5d1c..0000000 --- a/testdata/k8s/kubeconfig.yaml +++ /dev/null @@ -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