From a6363f1c1e65bbeb43c0abf21e2be15d6cc019a1 Mon Sep 17 00:00:00 2001
From: Ruslan Aliev <raliev@mirantis.com>
Date: Wed, 7 Oct 2020 22:58:00 -0500
Subject: [PATCH] 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 | 10 ++-
pkg/bootstrap/isogen/command.go | 66 +------------------
pkg/bootstrap/isogen/command_test.go | 66 -------------------
pkg/bootstrap/isogen/executor.go | 4 +-
pkg/bootstrap/isogen/testdata/config/config | 34 ----------
.../isogen/testdata/config/kubeconfig | 17 -----
.../bootstrap/image_configuration.yaml | 12 ----
.../ephemeral/bootstrap/kustomization.yaml | 2 -
.../bootstrap/image_configuration.yaml | 10 ---
.../ephemeral/bootstrap/kustomization.yaml | 2 -
.../bootstrap/image_configuration.yaml | 11 ----
.../ephemeral/bootstrap/kustomization.yaml | 2 -
pkg/phase/command.go | 3 +-
pkg/phase/ifc/executor.go | 3 +-
testdata/k8s/kubeconfig.yaml | 17 -----
15 files changed, 14 insertions(+), 245 deletions(-)
delete mode 100644 pkg/bootstrap/isogen/testdata/config/config
delete mode 100644 pkg/bootstrap/isogen/testdata/config/kubeconfig
delete mode 100644 pkg/bootstrap/isogen/testdata/missingkinddoc/site/test-site/ephemeral/bootstrap/image_configuration.yaml
delete mode 100644 pkg/bootstrap/isogen/testdata/missingkinddoc/site/test-site/ephemeral/bootstrap/kustomization.yaml
delete mode 100644 pkg/bootstrap/isogen/testdata/missingmetadoc/site/test-site/ephemeral/bootstrap/image_configuration.yaml
delete mode 100644 pkg/bootstrap/isogen/testdata/missingmetadoc/site/test-site/ephemeral/bootstrap/kustomization.yaml
delete mode 100644 pkg/bootstrap/isogen/testdata/missingvoldoc/site/test-site/ephemeral/bootstrap/image_configuration.yaml
delete mode 100644 pkg/bootstrap/isogen/testdata/missingvoldoc/site/test-site/ephemeral/bootstrap/kustomization.yaml
delete mode 100644 testdata/k8s/kubeconfig.yaml
diff --git a/cmd/image/build.go b/cmd/image/build.go
index f6ecd6856..dff470d18 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 3fb341bbf..bca644956 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 a3ff53bb5..5b7a9c677 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 ead0e9053..9427bd879 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 2046db657..000000000
--- 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 0fc12972f..000000000
--- 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 aecfb71f0..000000000
--- 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 86ef7db29..000000000
--- 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 91e86a591..000000000
--- 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 86ef7db29..000000000
--- 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 cf65e1c06..000000000
--- 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 86ef7db29..000000000
--- 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 17f9236a8..a2c915cdd 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 a1f6f8fde..c7da6a448 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 43c5d1c72..000000000
--- 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