From b799edc0a73e61fd6b776b94c87bc97403540f76 Mon Sep 17 00:00:00 2001 From: Ruslan Aliev Date: Tue, 8 Dec 2020 22:08:53 -0600 Subject: [PATCH] Split isogen package into isogen and isogen_test We should follow black-box pattern in unit test, so they should be performed in a separate package. This patch fixes it for isogen package. Change-Id: I60a22213e943eaa85ce75979071897c42308e1ae Signed-off-by: Ruslan Aliev Relates-To: #432 --- pkg/bootstrap/isogen/command.go | 55 ++++++++++++++------------- pkg/bootstrap/isogen/command_test.go | 21 +++++----- pkg/bootstrap/isogen/executor.go | 32 ++++++++-------- pkg/bootstrap/isogen/executor_test.go | 14 +++---- 4 files changed, 63 insertions(+), 59 deletions(-) diff --git a/pkg/bootstrap/isogen/command.go b/pkg/bootstrap/isogen/command.go index bca644956..819ceaa7c 100644 --- a/pkg/bootstrap/isogen/command.go +++ b/pkg/bootstrap/isogen/command.go @@ -53,18 +53,19 @@ const ( // BootstrapIsoOptions are used to generate bootstrap ISO type BootstrapIsoOptions struct { - docBundle document.Bundle - builder container.Container - doc document.Document - cfg *v1alpha1.ImageConfiguration + DocBundle document.Bundle + Builder container.Container + Doc document.Document + Cfg *v1alpha1.ImageConfiguration // optional fields for verbose output - debug bool - progress bool - writer io.Writer + Debug bool + Progress bool + Writer io.Writer } -func verifyInputs(cfg *v1alpha1.ImageConfiguration) error { +// VerifyInputs verifies image configuration +func VerifyInputs(cfg *v1alpha1.ImageConfiguration) error { if cfg.Container.Volume == "" { return config.ErrMissingConfig{ What: "Must specify volume bind for ISO builder container", @@ -111,25 +112,26 @@ func verifyArtifacts(cfg *v1alpha1.ImageConfiguration) error { return err } -func (opts BootstrapIsoOptions) createBootstrapIso() error { - cntVol := strings.Split(opts.cfg.Container.Volume, ":")[1] +// CreateBootstrapIso prepares and runs appropriate container to create a bootstrap ISO +func (opts BootstrapIsoOptions) CreateBootstrapIso() error { + cntVol := strings.Split(opts.Cfg.Container.Volume, ":")[1] log.Print("Creating cloud-init for ephemeral K8s") - userData, netConf, err := cloudinit.GetCloudData(opts.docBundle) + userData, netConf, err := cloudinit.GetCloudData(opts.DocBundle) if err != nil { return err } - builderCfgYaml, err := opts.doc.AsYAML() + builderCfgYaml, err := opts.Doc.AsYAML() if err != nil { return err } - fls := getContainerCfg(opts.cfg, builderCfgYaml, userData, netConf) + fls := getContainerCfg(opts.Cfg, builderCfgYaml, userData, netConf) if err = util.WriteFiles(fls, 0600); err != nil { return err } - vols := []string{opts.cfg.Container.Volume} + vols := []string{opts.Cfg.Container.Volume} builderCfgLocation := filepath.Join(cntVol, builderConfigFileName) log.Printf("Running default container command. Mounted dir: %s", vols) @@ -142,28 +144,28 @@ func (opts BootstrapIsoOptions) createBootstrapIso() error { fmt.Sprintf("NO_PROXY=%s", os.Getenv("NO_PROXY")), } - err = opts.builder.RunCommand([]string{}, nil, vols, envVars) + err = opts.Builder.RunCommand([]string{}, nil, vols, envVars) if err != nil { return err } log.Print("ISO generation is in progress. The whole process could take up to several minutes, please wait...") - if opts.debug || opts.progress { + if opts.Debug || opts.Progress { var cLogs io.ReadCloser - cLogs, err = opts.builder.GetContainerLogs() + cLogs, err = opts.Builder.GetContainerLogs() if err != nil { log.Printf("failed to read container logs %s", err) } else { switch { - case opts.progress: - if err = showProgress(cLogs, opts.writer); err != nil { + case opts.Progress: + if err = ShowProgress(cLogs, opts.Writer); err != nil { log.Debugf("the following error occurred while showing progress bar: %s", err.Error()) } - case opts.debug: + case opts.Debug: log.Print("start reading container logs") // either container log output or progress bar will be shown - if _, err = io.Copy(opts.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") @@ -171,21 +173,22 @@ func (opts BootstrapIsoOptions) createBootstrapIso() error { } } - if err = opts.builder.WaitUntilFinished(); err != nil { + if err = opts.Builder.WaitUntilFinished(); err != nil { return err } log.Print("ISO successfully built.") - if !opts.debug { + if !opts.Debug { log.Print("Removing container.") - return opts.builder.RmContainer() + return opts.Builder.RmContainer() } - log.Debugf("Debug flag is set. Container %s stopped but not deleted.", opts.builder.GetID()) + log.Debugf("Debug flag is set. Container %s stopped but not deleted.", opts.Builder.GetID()) return nil } -func showProgress(reader io.ReadCloser, writer io.Writer) error { +// ShowProgress prints progress bar during bootstrap ISO preparation +func ShowProgress(reader io.ReadCloser, writer io.Writer) error { reFindActions := regexp.MustCompile(reInstallActions) reBeginInstall := regexp.MustCompile(reInstallBegin) reFinishInstall := regexp.MustCompile(reInstallFinish) diff --git a/pkg/bootstrap/isogen/command_test.go b/pkg/bootstrap/isogen/command_test.go index 281235148..2759a0351 100644 --- a/pkg/bootstrap/isogen/command_test.go +++ b/pkg/bootstrap/isogen/command_test.go @@ -12,7 +12,7 @@ limitations under the License. */ -package isogen +package isogen_test import ( "bytes" @@ -26,6 +26,7 @@ import ( "github.com/stretchr/testify/require" api "opendev.org/airship/airshipctl/pkg/api/v1alpha1" + "opendev.org/airship/airshipctl/pkg/bootstrap/isogen" "opendev.org/airship/airshipctl/pkg/config" "opendev.org/airship/airshipctl/pkg/document" "opendev.org/airship/airshipctl/pkg/log" @@ -134,14 +135,14 @@ func TestBootstrapIso(t *testing.T) { for _, tt := range tests { outBuf := &bytes.Buffer{} log.Init(tt.debug, outBuf) - bootstrapOpts := BootstrapIsoOptions{ - docBundle: bundle, - builder: tt.builder, - doc: tt.doc, - cfg: tt.cfg, - debug: tt.debug, + bootstrapOpts := isogen.BootstrapIsoOptions{ + DocBundle: bundle, + Builder: tt.builder, + Doc: tt.doc, + Cfg: tt.cfg, + Debug: tt.debug, } - actualErr := bootstrapOpts.createBootstrapIso() + actualErr := bootstrapOpts.CreateBootstrapIso() actualOut := outBuf.String() for _, line := range tt.expectedOut { @@ -216,7 +217,7 @@ func TestVerifyInputs(t *testing.T) { for _, tt := range tests { tt := tt t.Run(tt.name, func(subTest *testing.T) { - actualErr := verifyInputs(tt.cfg) + actualErr := isogen.VerifyInputs(tt.cfg) assert.Equal(subTest, tt.expectedErr, actualErr) }) } @@ -242,7 +243,7 @@ func TestShowProgress(t *testing.T) { require.NoError(t, err) reader := ioutil.NopCloser(bytes.NewReader(testInput)) writer := bytes.NewBuffer(nil) - err = showProgress(reader, writer) + err = isogen.ShowProgress(reader, writer) require.NoError(t, err) assert.Contains(t, writer.String(), "Completed") } diff --git a/pkg/bootstrap/isogen/executor.go b/pkg/bootstrap/isogen/executor.go index bede44c53..683080885 100644 --- a/pkg/bootstrap/isogen/executor.go +++ b/pkg/bootstrap/isogen/executor.go @@ -36,8 +36,8 @@ type Executor struct { ExecutorBundle document.Bundle ExecutorDocument document.Document - imgConf *v1alpha1.ImageConfiguration - builder container.Container + ImgConf *v1alpha1.ImageConfiguration + Builder container.Container } // RegisterExecutor adds executor to phase executor registry @@ -70,7 +70,7 @@ func NewExecutor(cfg ifc.ExecutorConfig) (ifc.Executor, error) { return &Executor{ ExecutorBundle: bundle, ExecutorDocument: cfg.ExecutorDocument, - imgConf: apiObj, + ImgConf: apiObj, }, nil } @@ -96,13 +96,13 @@ func (c *Executor) Run(evtCh chan events.Event, opts ifc.RunOptions) { return } - if c.builder == nil { + if c.Builder == nil { ctx := context.Background() builder, err := container.NewContainer( ctx, - c.imgConf.Container.ContainerRuntime, - c.imgConf.Container.Image) - c.builder = builder + c.ImgConf.Container.ContainerRuntime, + c.ImgConf.Container.Image) + c.Builder = builder if err != nil { handleError(evtCh, err) return @@ -110,16 +110,16 @@ func (c *Executor) Run(evtCh chan events.Event, opts ifc.RunOptions) { } bootstrapOpts := BootstrapIsoOptions{ - docBundle: c.ExecutorBundle, - builder: c.builder, - doc: c.ExecutorDocument, - cfg: c.imgConf, - debug: log.DebugEnabled(), - progress: opts.Progress, - writer: log.Writer(), + DocBundle: c.ExecutorBundle, + Builder: c.Builder, + Doc: c.ExecutorDocument, + Cfg: c.ImgConf, + Debug: log.DebugEnabled(), + Progress: opts.Progress, + Writer: log.Writer(), } - err := bootstrapOpts.createBootstrapIso() + err := bootstrapOpts.CreateBootstrapIso() if err != nil { handleError(evtCh, err) return @@ -129,7 +129,7 @@ func (c *Executor) Run(evtCh chan events.Event, opts ifc.RunOptions) { Operation: events.IsogenValidation, Message: "image is generated successfully, verifying artifacts", }) - err = verifyArtifacts(c.imgConf) + err = verifyArtifacts(c.ImgConf) if err != nil { handleError(evtCh, err) return diff --git a/pkg/bootstrap/isogen/executor_test.go b/pkg/bootstrap/isogen/executor_test.go index e81426919..62c3e21f6 100644 --- a/pkg/bootstrap/isogen/executor_test.go +++ b/pkg/bootstrap/isogen/executor_test.go @@ -12,7 +12,7 @@ limitations under the License. */ -package isogen +package isogen_test import ( "testing" @@ -20,10 +20,10 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "k8s.io/apimachinery/pkg/runtime/schema" "opendev.org/airship/airshipctl/pkg/api/v1alpha1" + "opendev.org/airship/airshipctl/pkg/bootstrap/isogen" "opendev.org/airship/airshipctl/pkg/container" "opendev.org/airship/airshipctl/pkg/document" "opendev.org/airship/airshipctl/pkg/events" @@ -59,7 +59,7 @@ func TestRegisterExecutor(t *testing.T) { Version: "v1alpha1", Kind: "ImageConfiguration", } - err := RegisterExecutor(registry) + err := isogen.RegisterExecutor(registry) require.NoError(t, err) _, found := registry[expectedGVK] @@ -69,7 +69,7 @@ func TestRegisterExecutor(t *testing.T) { func TestNewExecutor(t *testing.T) { execDoc, err := document.NewDocumentFromBytes([]byte(executorDoc)) require.NoError(t, err) - _, err = NewExecutor(ifc.ExecutorConfig{ + _, err = isogen.NewExecutor(ifc.ExecutorConfig{ ExecutorDocument: execDoc, BundleFactory: testBundleFactory(executorBundlePath)}) require.NoError(t, err) @@ -144,11 +144,11 @@ func TestExecutorRun(t *testing.T) { for _, test := range testCases { tt := test t.Run(tt.name, func(t *testing.T) { - executor := &Executor{ + executor := &isogen.Executor{ ExecutorDocument: testDoc, ExecutorBundle: bundle, - imgConf: testCfg, - builder: tt.builder, + ImgConf: testCfg, + Builder: tt.builder, } ch := make(chan events.Event) go executor.Run(ch, ifc.RunOptions{})