Skip to content

Commit

Permalink
breaking change: unify disk-virt task code and rename their parameters
Browse files Browse the repository at this point in the history
The disk-virt tasks have almost identical code. This
commit keeps only single version of disk-virt code.
The params of the tasks were unified and renamed:
customizeCommands, sysprepCommands -> virtCommands
additionalOptions -> additionalVirtOptions

Signed-off-by: Karel Simon <[email protected]>
  • Loading branch information
ksimon1 committed Jul 10, 2024
1 parent b1eed4d commit e585720
Show file tree
Hide file tree
Showing 31 changed files with 156 additions and 164 deletions.
10 changes: 5 additions & 5 deletions cmd/disk-virt-customize/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@ package main

import (
goarg "github.com/alexflint/go-arg"
. "github.com/kubevirt/kubevirt-tekton-tasks/modules/disk-virt-customize/pkg/constants"
"github.com/kubevirt/kubevirt-tekton-tasks/modules/disk-virt-customize/pkg/execute"
"github.com/kubevirt/kubevirt-tekton-tasks/modules/disk-virt-customize/pkg/utils/log"
"github.com/kubevirt/kubevirt-tekton-tasks/modules/disk-virt-customize/pkg/utils/parse"
. "github.com/kubevirt/kubevirt-tekton-tasks/modules/disk-virt/pkg/constants"
"github.com/kubevirt/kubevirt-tekton-tasks/modules/disk-virt/pkg/execute"
"github.com/kubevirt/kubevirt-tekton-tasks/modules/disk-virt/pkg/utils/log"
"github.com/kubevirt/kubevirt-tekton-tasks/modules/disk-virt/pkg/utils/parse"
"github.com/kubevirt/kubevirt-tekton-tasks/modules/shared/pkg/exit"
"go.uber.org/zap"
)
Expand All @@ -23,7 +23,7 @@ func main() {
if err := cliOptions.Init(); err != nil {
exit.ExitOrDieFromError(InvalidArguments, err)
}
executor := execute.NewExecutor(cliOptions, DiskImagePath)
executor := execute.NewExecutor(cliOptions, DiskImagePath, "virt-customize")

if err := executor.PrepareGuestFSAppliance(); err != nil {
exit.ExitOrDieFromError(PrepareGuestFSApplianceFailed, err)
Expand Down
10 changes: 5 additions & 5 deletions cmd/disk-virt-sysprep/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@ package main

import (
goarg "github.com/alexflint/go-arg"
. "github.com/kubevirt/kubevirt-tekton-tasks/modules/disk-virt-sysprep/pkg/constants"
"github.com/kubevirt/kubevirt-tekton-tasks/modules/disk-virt-sysprep/pkg/execute"
"github.com/kubevirt/kubevirt-tekton-tasks/modules/disk-virt-sysprep/pkg/utils/log"
"github.com/kubevirt/kubevirt-tekton-tasks/modules/disk-virt-sysprep/pkg/utils/parse"
. "github.com/kubevirt/kubevirt-tekton-tasks/modules/disk-virt/pkg/constants"
"github.com/kubevirt/kubevirt-tekton-tasks/modules/disk-virt/pkg/execute"
"github.com/kubevirt/kubevirt-tekton-tasks/modules/disk-virt/pkg/utils/log"
"github.com/kubevirt/kubevirt-tekton-tasks/modules/disk-virt/pkg/utils/parse"
"github.com/kubevirt/kubevirt-tekton-tasks/modules/shared/pkg/exit"
"go.uber.org/zap"
)
Expand All @@ -23,7 +23,7 @@ func main() {
if err := cliOptions.Init(); err != nil {
exit.ExitOrDieFromError(InvalidArguments, err)
}
executor := execute.NewExecutor(cliOptions, DiskImagePath)
executor := execute.NewExecutor(cliOptions, DiskImagePath, "virt-sysprep")

if err := executor.PrepareGuestFSAppliance(); err != nil {
exit.ExitOrDieFromError(PrepareGuestFSApplianceFailed, err)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ spec:
params:
- name: pvc
value: "$(tasks.modify-data-object.results.name)"
- name: customizeCommands
- name: virtCommands
value: |
install git,vim,pip
run-command pip install flask
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,10 @@ spec:
params:
- name: pvc
value: "$(tasks.modify-data-object.results.name)"
- name: sysprepCommands
- name: virtCommands
value: |
run-command yum update -y
- name: additionalOptions
- name: additionalVirtOptions
value: "--network"
taskRef:
kind: Task
Expand Down
44 changes: 0 additions & 44 deletions modules/disk-virt-sysprep/pkg/utils/parse/clioptions.go

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const (
const (
DiskImagePath = "/mnt/targetpvc/disk.img"
GuestFSApplianceArchivePath = "/usr/local/lib/guestfs/appliance"
VirtSysprepCommandsFileName = "virt_sysprep_commands"
VirtCommandsFileName = "virt_commands"
GuestFSApplianceArchivePathEnv = "LIBGUESTFS_APPLIANCE_ARCHIVE_PATH"
DiskVirtSysprepHome = "/home/disk-virt-sysprep"
DiskVirtHome = "/home/disk-virt"
)
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package execute_test
import (
"testing"

"github.com/kubevirt/kubevirt-tekton-tasks/modules/disk-virt-sysprep/pkg/utilstest"
"github.com/kubevirt/kubevirt-tekton-tasks/modules/disk-virt/pkg/utilstest"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ import (
"os/exec"
"strings"

"github.com/kubevirt/kubevirt-tekton-tasks/modules/disk-virt-sysprep/pkg/constants"
"github.com/kubevirt/kubevirt-tekton-tasks/modules/disk-virt-sysprep/pkg/utils/log"
"github.com/kubevirt/kubevirt-tekton-tasks/modules/disk-virt-sysprep/pkg/utils/parse"
"github.com/kubevirt/kubevirt-tekton-tasks/modules/disk-virt/pkg/constants"
"github.com/kubevirt/kubevirt-tekton-tasks/modules/disk-virt/pkg/utils/log"
"github.com/kubevirt/kubevirt-tekton-tasks/modules/disk-virt/pkg/utils/parse"
"github.com/kubevirt/kubevirt-tekton-tasks/modules/shared/pkg/env"
"github.com/kubevirt/kubevirt-tekton-tasks/modules/shared/pkg/exit"
"github.com/kubevirt/kubevirt-tekton-tasks/modules/shared/pkg/options"
Expand All @@ -18,10 +18,11 @@ import (
type Executor struct {
cliOptions *parse.CLIOptions
diskImagePath string
command string
}

func NewExecutor(clioptions *parse.CLIOptions, diskImagePath string) *Executor {
return &Executor{cliOptions: clioptions, diskImagePath: diskImagePath}
func NewExecutor(clioptions *parse.CLIOptions, diskImagePath, command string) *Executor {
return &Executor{cliOptions: clioptions, diskImagePath: diskImagePath, command: command}
}

func (e *Executor) PrepareGuestFSAppliance() error {
Expand All @@ -37,7 +38,7 @@ func (e *Executor) PrepareGuestFSAppliance() error {
}

func (e *Executor) Execute() error {
virtSysprepScriptFileName, err := writeToTmpFile(e.cliOptions.GetSysprepCommands())
virtScriptFileName, err := writeToTmpFile(e.cliOptions.GetCommands())
if err != nil {
return err
}
Expand All @@ -46,18 +47,18 @@ func (e *Executor) Execute() error {
"--add",
e.diskImagePath,
"--commands-from-file",
virtSysprepScriptFileName,
virtScriptFileName,
})

additionalVirtSysprepOpts, err := options.NewCommandOptions(e.cliOptions.GetAdditionalVirtSysprepOptions())
additionalVirtOpts, err := options.NewCommandOptions(e.cliOptions.GetAdditionalVirtOptions())
if err != nil {
return err
}
opts.AddOptions(additionalVirtSysprepOpts.GetAll()...)
SetupVirtSysprepOptions(opts, e.cliOptions)
opts.AddOptions(additionalVirtOpts.GetAll()...)
SetupVirtOptions(opts, e.cliOptions)

log.GetLogger().Debug("executing virt-sysprep command with options: " + strings.Join(opts.GetAll(), " "))
cmd := exec.Command("virt-sysprep", opts.GetAll()...)
log.GetLogger().Debug("executing virt command with options: " + strings.Join(opts.GetAll(), " "))
cmd := exec.Command(e.command, opts.GetAll()...)
cmd.Env = os.Environ()
cmd.Stdout = os.Stdout
cmd.Stderr = os.Stderr
Expand All @@ -76,7 +77,7 @@ func (e *Executor) Execute() error {
}

func writeToTmpFile(content string) (string, error) {
f, err := ioutil.TempFile("", constants.VirtSysprepCommandsFileName)
f, err := ioutil.TempFile("", constants.VirtCommandsFileName)
if err != nil {
return "", err
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
package execute

import (
"github.com/kubevirt/kubevirt-tekton-tasks/modules/disk-virt-sysprep/pkg/utils/parse"
"github.com/kubevirt/kubevirt-tekton-tasks/modules/disk-virt/pkg/utils/parse"
"github.com/kubevirt/kubevirt-tekton-tasks/modules/shared/pkg/options"
)

func SetupVirtSysprepOptions(opts *options.CommandOptions, inputCliOptions *parse.CLIOptions) {
func SetupVirtOptions(opts *options.CommandOptions, inputCliOptions *parse.CLIOptions) {
if inputCliOptions.IsVerbose() {
if !opts.IncludesOption("-v") && !opts.IncludesOption("--verbose") {
opts.AddFlag("--verbose")
Expand Down
Original file line number Diff line number Diff line change
@@ -1,39 +1,39 @@
package execute_test

import (
"github.com/kubevirt/kubevirt-tekton-tasks/modules/disk-virt-sysprep/pkg/execute"
"github.com/kubevirt/kubevirt-tekton-tasks/modules/disk-virt-sysprep/pkg/utils/parse"
"github.com/kubevirt/kubevirt-tekton-tasks/modules/disk-virt/pkg/execute"
"github.com/kubevirt/kubevirt-tekton-tasks/modules/disk-virt/pkg/utils/parse"
"github.com/kubevirt/kubevirt-tekton-tasks/modules/shared/pkg/options"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)

var _ = Describe("SetupVirtSysprepOptions", func() {
var _ = Describe("SetupVirtOptions", func() {
DescribeTable("sets options correctly", func(inputCliOptions *parse.CLIOptions, expected []string) {
Expect(inputCliOptions.Init()).Should(Succeed())

opts, err := options.NewCommandOptions(inputCliOptions.AdditionalVirtSysprepOptions)
opts, err := options.NewCommandOptions(inputCliOptions.AdditionalVirtOptions)
Expect(err).Should(Succeed())

execute.SetupVirtSysprepOptions(opts, inputCliOptions)
execute.SetupVirtOptions(opts, inputCliOptions)
Expect(opts.GetAll()).Should(Equal(expected))
},
Entry("empty", &parse.CLIOptions{
SysprepCommands: "update",
Commands: "update",
},
[]string{},
),
Entry("verbose false does not change anything verbose cli arguments", &parse.CLIOptions{
SysprepCommands: "update",
AdditionalVirtSysprepOptions: "--network --dry-run",
Verbose: "false",
Commands: "update",
AdditionalVirtOptions: "--network --dry-run",
Verbose: "false",
}, []string{
"--network", "--dry-run",
}),
Entry("verbose adds only one argument", &parse.CLIOptions{
SysprepCommands: "update",
AdditionalVirtSysprepOptions: "--network --dry-run -q -v",
Verbose: "true",
Commands: "update",
AdditionalVirtOptions: "--network --dry-run -q -v",
Verbose: "true",
}, []string{
"--network",
"--dry-run",
Expand All @@ -42,8 +42,8 @@ var _ = Describe("SetupVirtSysprepOptions", func() {
"-x",
}),
Entry("verbose adds both arguments", &parse.CLIOptions{
SysprepCommands: "update",
Verbose: "true",
Commands: "update",
Verbose: "true",
}, []string{
"--verbose",
"-x",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package log_test

import (
"github.com/kubevirt/kubevirt-tekton-tasks/modules/disk-virt-sysprep/pkg/utils/log"
"github.com/kubevirt/kubevirt-tekton-tasks/modules/disk-virt/pkg/utils/log"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"go.uber.org/zap/zapcore"
Expand Down
File renamed without changes.
44 changes: 44 additions & 0 deletions modules/disk-virt/pkg/utils/parse/clioptions.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package parse

import (
"github.com/kubevirt/kubevirt-tekton-tasks/modules/shared/pkg/zutils"
"go.uber.org/zap/zapcore"
)

const (
commandsOptionName = "virt-commands"
commandsEnvVarName = "VIRT_COMMANDS"
)

type CLIOptions struct {
Commands string `arg:"--virt-commands,env:VIRT_COMMANDS" placeholder:"VIRT_COMMANDS" help:"virt script in --commands-from-file format to execute on target pvc."`
AdditionalVirtOptions string `arg:"--additional-virt-options,env:ADDITIONAL_VIRT_OPTIONS" placeholder:"OPTIONS" help:"additional options to pass to virt command"`
Verbose string `arg:"--verbose" placeholder:"true|false" help:"Enable verbose mode and tracing of libguestfs API calls."`
}

func (c *CLIOptions) GetDebugLevel() zapcore.Level {
if c.IsVerbose() {
return zapcore.DebugLevel
}
return zapcore.InfoLevel
}

func (c *CLIOptions) IsVerbose() bool {
return zutils.IsTrue(c.Verbose)
}

func (c *CLIOptions) GetCommands() string {
return c.Commands
}

func (c *CLIOptions) GetAdditionalVirtOptions() string {
return c.AdditionalVirtOptions
}

func (c *CLIOptions) Init() error {
if err := c.validateCommands(); err != nil {
return err
}

return nil
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package parse_test
import (
"reflect"

"github.com/kubevirt/kubevirt-tekton-tasks/modules/disk-virt-sysprep/pkg/utils/parse"
"github.com/kubevirt/kubevirt-tekton-tasks/modules/disk-virt/pkg/utils/parse"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"go.uber.org/zap/zapcore"
Expand All @@ -20,7 +20,7 @@ var _ = Describe("CLIOptions", func() {
DescribeTable("Init return correct assertion errors", func(expectedErrMessage string, options *parse.CLIOptions) {
Expect(options.Init().Error()).To(ContainSubstring(expectedErrMessage))
},
Entry("no sysprep commands", "sysprep-commands option or SYSPREP_COMMANDS env variable is required", &parse.CLIOptions{}),
Entry("no sysprep commands", "virt-commands option or VIRT_COMMANDS env variable is required", &parse.CLIOptions{}),
)
DescribeTable("Parses and returns correct values", func(options *parse.CLIOptions, expectedOptions map[string]interface{}) {
Expect(options.Init()).Should(Succeed())
Expand All @@ -31,22 +31,22 @@ var _ = Describe("CLIOptions", func() {
}
},
Entry("returns valid defaults", &parse.CLIOptions{
SysprepCommands: "test",
Commands: "test",
}, map[string]interface{}{
"GetSysprepCommands": "test",
"GetAdditionalVirtSysprepOptions": "",
"GetDebugLevel": zapcore.InfoLevel,
"IsVerbose": false,
"GetCommands": "test",
"GetAdditionalVirtOptions": "",
"GetDebugLevel": zapcore.InfoLevel,
"IsVerbose": false,
}),
Entry("handles cli arguments", &parse.CLIOptions{
SysprepCommands: sysprepCommands,
AdditionalVirtSysprepOptions: "--network --dry-run -q -v",
Verbose: "true",
Commands: sysprepCommands,
AdditionalVirtOptions: "--network --dry-run -q -v",
Verbose: "true",
}, map[string]interface{}{
"GetSysprepCommands": sysprepCommands,
"GetAdditionalVirtSysprepOptions": "--network --dry-run -q -v",
"GetDebugLevel": zapcore.DebugLevel,
"IsVerbose": true,
"GetCommands": sysprepCommands,
"GetAdditionalVirtOptions": "--network --dry-run -q -v",
"GetDebugLevel": zapcore.DebugLevel,
"IsVerbose": true,
}),
)
})
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package parse_test
import (
"testing"

"github.com/kubevirt/kubevirt-tekton-tasks/modules/disk-virt-sysprep/pkg/utilstest"
"github.com/kubevirt/kubevirt-tekton-tasks/modules/disk-virt/pkg/utilstest"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
Expand Down
Loading

0 comments on commit e585720

Please sign in to comment.