From 99fadefa2213c8b24ba575fa582ae984857fc42e Mon Sep 17 00:00:00 2001 From: Martin Pywell Date: Fri, 27 Sep 2024 00:39:55 +0000 Subject: [PATCH] add skip_convert_to_template option --- .web-docs/components/builder/clone/README.md | 3 ++ .web-docs/components/builder/iso/README.md | 3 ++ builder/proxmox/clone/config.hcl2spec.go | 2 + builder/proxmox/common/artifact.go | 11 +++-- builder/proxmox/common/builder.go | 17 ++++--- builder/proxmox/common/config.go | 3 ++ builder/proxmox/common/config.hcl2spec.go | 2 + .../common/step_convert_to_template.go | 28 +++++++---- .../common/step_convert_to_template_test.go | 48 +++++++++++++++---- ...late_config.go => step_finalize_config.go} | 12 ++--- ...g_test.go => step_finalize_config_test.go} | 4 +- builder/proxmox/iso/config.hcl2spec.go | 2 + .../proxmox/common/Config-not-required.mdx | 3 ++ 13 files changed, 99 insertions(+), 39 deletions(-) rename builder/proxmox/common/{step_finalize_template_config.go => step_finalize_config.go} (92%) rename builder/proxmox/common/{step_finalize_template_config_test.go => step_finalize_config_test.go} (98%) diff --git a/.web-docs/components/builder/clone/README.md b/.web-docs/components/builder/clone/README.md index a60f5488..21b74379 100644 --- a/.web-docs/components/builder/clone/README.md +++ b/.web-docs/components/builder/clone/README.md @@ -262,6 +262,9 @@ boot time. - `template_description` (string) - Description of the template, visible in the Proxmox interface. +- `skip_convert_to_template` (bool) - Skip converting the VM to a template on completion of build. + Defaults to `false` + - `cloud_init` (bool) - If true, add an empty Cloud-Init CDROM drive after the virtual machine has been converted to a template. Defaults to `false`. diff --git a/.web-docs/components/builder/iso/README.md b/.web-docs/components/builder/iso/README.md index cd806e81..be7c6d57 100644 --- a/.web-docs/components/builder/iso/README.md +++ b/.web-docs/components/builder/iso/README.md @@ -197,6 +197,9 @@ in the image's Cloud-Init settings for provisioning. - `template_description` (string) - Description of the template, visible in the Proxmox interface. +- `skip_convert_to_template` (bool) - Skip converting the VM to a template on completion of build. + Defaults to `false` + - `cloud_init` (bool) - If true, add an empty Cloud-Init CDROM drive after the virtual machine has been converted to a template. Defaults to `false`. diff --git a/builder/proxmox/clone/config.hcl2spec.go b/builder/proxmox/clone/config.hcl2spec.go index e131cb6b..8fdf2f9a 100644 --- a/builder/proxmox/clone/config.hcl2spec.go +++ b/builder/proxmox/clone/config.hcl2spec.go @@ -114,6 +114,7 @@ type FlatConfig struct { DisableKVM *bool `mapstructure:"disable_kvm" cty:"disable_kvm" hcl:"disable_kvm"` TemplateName *string `mapstructure:"template_name" cty:"template_name" hcl:"template_name"` TemplateDescription *string `mapstructure:"template_description" cty:"template_description" hcl:"template_description"` + SkipConvertToTemplate *bool `mapstructure:"skip_convert_to_template" cty:"skip_convert_to_template" hcl:"skip_convert_to_template"` CloudInit *bool `mapstructure:"cloud_init" cty:"cloud_init" hcl:"cloud_init"` CloudInitStoragePool *string `mapstructure:"cloud_init_storage_pool" cty:"cloud_init_storage_pool" hcl:"cloud_init_storage_pool"` CloudInitDiskType *string `mapstructure:"cloud_init_disk_type" cty:"cloud_init_disk_type" hcl:"cloud_init_disk_type"` @@ -243,6 +244,7 @@ func (*FlatConfig) HCL2Spec() map[string]hcldec.Spec { "disable_kvm": &hcldec.AttrSpec{Name: "disable_kvm", Type: cty.Bool, Required: false}, "template_name": &hcldec.AttrSpec{Name: "template_name", Type: cty.String, Required: false}, "template_description": &hcldec.AttrSpec{Name: "template_description", Type: cty.String, Required: false}, + "skip_convert_to_template": &hcldec.AttrSpec{Name: "skip_convert_to_template", Type: cty.Bool, Required: false}, "cloud_init": &hcldec.AttrSpec{Name: "cloud_init", Type: cty.Bool, Required: false}, "cloud_init_storage_pool": &hcldec.AttrSpec{Name: "cloud_init_storage_pool", Type: cty.String, Required: false}, "cloud_init_disk_type": &hcldec.AttrSpec{Name: "cloud_init_disk_type", Type: cty.String, Required: false}, diff --git a/builder/proxmox/common/artifact.go b/builder/proxmox/common/artifact.go index f9565f31..fcb5c704 100644 --- a/builder/proxmox/common/artifact.go +++ b/builder/proxmox/common/artifact.go @@ -14,7 +14,8 @@ import ( type Artifact struct { builderID string - templateID int + artifactID int + artifactType string proxmoxClient *proxmox.Client // StateData should store data such as GeneratedData @@ -34,11 +35,11 @@ func (*Artifact) Files() []string { } func (a *Artifact) Id() string { - return strconv.Itoa(a.templateID) + return strconv.Itoa(a.artifactID) } func (a *Artifact) String() string { - return fmt.Sprintf("A template was created: %d", a.templateID) + return fmt.Sprintf("A %s was created: %d", a.artifactType, a.artifactID) } func (a *Artifact) State(name string) interface{} { @@ -46,7 +47,7 @@ func (a *Artifact) State(name string) interface{} { } func (a *Artifact) Destroy() error { - log.Printf("Destroying template: %d", a.templateID) - _, err := a.proxmoxClient.DeleteVm(proxmox.NewVmRef(a.templateID)) + log.Printf("Destroying %s: %d", a.artifactType, a.artifactID) + _, err := a.proxmoxClient.DeleteVm(proxmox.NewVmRef(a.artifactID)) return err } diff --git a/builder/proxmox/common/builder.go b/builder/proxmox/common/builder.go index 3bfb55d4..aa364bbe 100644 --- a/builder/proxmox/common/builder.go +++ b/builder/proxmox/common/builder.go @@ -71,7 +71,7 @@ func (b *Builder) Run(ctx context.Context, ui packersdk.Ui, hook packersdk.Hook, }, &stepRemoveCloudInitDrive{}, &stepConvertToTemplate{}, - &stepFinalizeTemplateConfig{}, + &stepFinalizeConfig{}, &stepSuccess{}, } preSteps := b.preSteps @@ -118,19 +118,24 @@ func (b *Builder) Run(ctx context.Context, ui packersdk.Ui, hook packersdk.Hook, return nil, errors.New("build was cancelled") } - // Verify that the template_id was set properly, otherwise we didn't progress through the last step - tplID, ok := state.Get("template_id").(int) + // Verify that the artifact_id and artifact_type was set properly, otherwise we didn't progress through the last step + artifactID, ok := state.Get("artifact_id").(int) if !ok { - return nil, fmt.Errorf("template ID could not be determined") + return nil, fmt.Errorf("artifact ID could not be determined") + } + + artifactType, ok := state.Get("artifact_type").(string) + if !ok { + return nil, fmt.Errorf("artifact type could not be determined") } artifact := &Artifact{ builderID: b.id, - templateID: tplID, + artifactID: artifactID, + artifactType: artifactType, proxmoxClient: b.proxmoxClient, StateData: map[string]interface{}{"generated_data": state.Get("generated_data")}, } - return artifact, nil } diff --git a/builder/proxmox/common/config.go b/builder/proxmox/common/config.go index 2a29d944..c882a29c 100644 --- a/builder/proxmox/common/config.go +++ b/builder/proxmox/common/config.go @@ -178,6 +178,9 @@ type Config struct { // Description of the template, visible in // the Proxmox interface. TemplateDescription string `mapstructure:"template_description"` + // Skip converting the VM to a template on completion of build. + // Defaults to `false` + SkipConvertToTemplate bool `mapstructure:"skip_convert_to_template"` // If true, add an empty Cloud-Init CDROM drive after the virtual // machine has been converted to a template. Defaults to `false`. diff --git a/builder/proxmox/common/config.hcl2spec.go b/builder/proxmox/common/config.hcl2spec.go index cb619338..78441831 100644 --- a/builder/proxmox/common/config.hcl2spec.go +++ b/builder/proxmox/common/config.hcl2spec.go @@ -113,6 +113,7 @@ type FlatConfig struct { DisableKVM *bool `mapstructure:"disable_kvm" cty:"disable_kvm" hcl:"disable_kvm"` TemplateName *string `mapstructure:"template_name" cty:"template_name" hcl:"template_name"` TemplateDescription *string `mapstructure:"template_description" cty:"template_description" hcl:"template_description"` + SkipConvertToTemplate *bool `mapstructure:"skip_convert_to_template" cty:"skip_convert_to_template" hcl:"skip_convert_to_template"` CloudInit *bool `mapstructure:"cloud_init" cty:"cloud_init" hcl:"cloud_init"` CloudInitStoragePool *string `mapstructure:"cloud_init_storage_pool" cty:"cloud_init_storage_pool" hcl:"cloud_init_storage_pool"` CloudInitDiskType *string `mapstructure:"cloud_init_disk_type" cty:"cloud_init_disk_type" hcl:"cloud_init_disk_type"` @@ -236,6 +237,7 @@ func (*FlatConfig) HCL2Spec() map[string]hcldec.Spec { "disable_kvm": &hcldec.AttrSpec{Name: "disable_kvm", Type: cty.Bool, Required: false}, "template_name": &hcldec.AttrSpec{Name: "template_name", Type: cty.String, Required: false}, "template_description": &hcldec.AttrSpec{Name: "template_description", Type: cty.String, Required: false}, + "skip_convert_to_template": &hcldec.AttrSpec{Name: "skip_convert_to_template", Type: cty.Bool, Required: false}, "cloud_init": &hcldec.AttrSpec{Name: "cloud_init", Type: cty.Bool, Required: false}, "cloud_init_storage_pool": &hcldec.AttrSpec{Name: "cloud_init_storage_pool", Type: cty.String, Required: false}, "cloud_init_disk_type": &hcldec.AttrSpec{Name: "cloud_init_disk_type", Type: cty.String, Required: false}, diff --git a/builder/proxmox/common/step_convert_to_template.go b/builder/proxmox/common/step_convert_to_template.go index ac663341..43550820 100644 --- a/builder/proxmox/common/step_convert_to_template.go +++ b/builder/proxmox/common/step_convert_to_template.go @@ -16,7 +16,7 @@ import ( // stepConvertToTemplate takes the running VM configured in earlier steps, stops it, and // converts it into a Proxmox template. // -// It sets the template_id state which is used for Artifact lookup. +// It sets the artifact_id state which is used for Artifact lookup. type stepConvertToTemplate struct{} type templateConverter interface { @@ -30,6 +30,7 @@ func (s *stepConvertToTemplate) Run(ctx context.Context, state multistep.StateBa ui := state.Get("ui").(packersdk.Ui) client := state.Get("proxmoxClient").(templateConverter) vmRef := state.Get("vmRef").(*proxmox.VmRef) + c := state.Get("config").(*Config) ui.Say("Stopping VM") _, err := client.ShutdownVm(vmRef) @@ -40,16 +41,23 @@ func (s *stepConvertToTemplate) Run(ctx context.Context, state multistep.StateBa return multistep.ActionHalt } - ui.Say("Converting VM to template") - err = client.CreateTemplate(vmRef) - if err != nil { - err := fmt.Errorf("Error converting VM to template: %s", err) - state.Put("error", err) - ui.Error(err.Error()) - return multistep.ActionHalt + if c.SkipConvertToTemplate { + ui.Say("skip_convert_to_template set, skipping conversion to template") + state.Put("artifact_type", "VM") + } else { + ui.Say("Converting VM to template") + err = client.CreateTemplate(vmRef) + if err != nil { + err := fmt.Errorf("Error converting VM to template: %s", err) + state.Put("error", err) + ui.Error(err.Error()) + return multistep.ActionHalt + } + state.Put("artifact_type", "template") } - log.Printf("template_id: %d", vmRef.VmId()) - state.Put("template_id", vmRef.VmId()) + + log.Printf("artifact_id: %d", vmRef.VmId()) + state.Put("artifact_id", vmRef.VmId()) return multistep.ActionContinue } diff --git a/builder/proxmox/common/step_convert_to_template_test.go b/builder/proxmox/common/step_convert_to_template_test.go index 01835fc2..0bfe400d 100644 --- a/builder/proxmox/common/step_convert_to_template_test.go +++ b/builder/proxmox/common/step_convert_to_template_test.go @@ -34,27 +34,45 @@ func TestConvertToTemplate(t *testing.T) { expectCallCreateTemplate bool createTemplateErr error expectedAction multistep.StepAction - expectTemplateIdSet bool + expectArtifactIdSet bool + expectArtifactType string + builderConfig *Config }{ { - name: "no errors returns continue and sets template id", + name: "no errors returns continue and sets template artifact type", expectCallCreateTemplate: true, expectedAction: multistep.ActionContinue, - expectTemplateIdSet: true, + expectArtifactIdSet: true, + expectArtifactType: "template", + builderConfig: &Config{}, + }, + { + name: "no errors returns continue and sets vm artifact type", + expectCallCreateTemplate: true, + expectedAction: multistep.ActionContinue, + expectArtifactIdSet: true, + expectArtifactType: "VM", + builderConfig: &Config{ + SkipConvertToTemplate: true, + }, }, { name: "when shutdown fails, don't try to create template and halt", shutdownErr: fmt.Errorf("failed to stop vm"), expectCallCreateTemplate: false, expectedAction: multistep.ActionHalt, - expectTemplateIdSet: false, + expectArtifactIdSet: false, + expectArtifactType: "", + builderConfig: &Config{}, }, { name: "when create template fails, halt", expectCallCreateTemplate: true, createTemplateErr: fmt.Errorf("failed to stop vm"), expectedAction: multistep.ActionHalt, - expectTemplateIdSet: false, + expectArtifactIdSet: false, + expectArtifactType: "", + builderConfig: &Config{}, }, } @@ -85,6 +103,7 @@ func TestConvertToTemplate(t *testing.T) { state.Put("ui", packersdk.TestUi(t)) state.Put("vmRef", proxmox.NewVmRef(vmid)) state.Put("proxmoxClient", converter) + state.Put("config", c.builderConfig) step := stepConvertToTemplate{} action := step.Run(context.TODO(), state) @@ -92,14 +111,23 @@ func TestConvertToTemplate(t *testing.T) { t.Errorf("Expected action to be %v, got %v", c.expectedAction, action) } - id, wasSet := state.GetOk("template_id") + artifactId, artifactIdWasSet := state.GetOk("artifact_id") + artifactType, artifactTypeWasSet := state.GetOk("artifact_type") + + if c.expectArtifactIdSet != artifactIdWasSet { + t.Errorf("Expected artifact_id state present=%v was present=%v", c.expectArtifactIdSet, artifactIdWasSet) + } + + if c.expectArtifactIdSet && artifactId != vmid { + t.Errorf("Expected artifact_id state to be set to %d, got %v", vmid, artifactId) + } - if c.expectTemplateIdSet != wasSet { - t.Errorf("Expected template_id state present=%v was present=%v", c.expectTemplateIdSet, wasSet) + if c.expectArtifactType == "" && artifactTypeWasSet { + t.Errorf("Expected artifact_type state present=%v was present=%v", c.expectArtifactType, artifactTypeWasSet) } - if c.expectTemplateIdSet && id != vmid { - t.Errorf("Expected template_id state to be set to %d, got %v", vmid, id) + if artifactTypeWasSet && c.expectArtifactType != artifactType { + t.Errorf("Expected artifact_type state to be set to %s, got %s", c.expectArtifactType, artifactType) } }) } diff --git a/builder/proxmox/common/step_finalize_template_config.go b/builder/proxmox/common/step_finalize_config.go similarity index 92% rename from builder/proxmox/common/step_finalize_template_config.go rename to builder/proxmox/common/step_finalize_config.go index ff739043..f07625a9 100644 --- a/builder/proxmox/common/step_finalize_template_config.go +++ b/builder/proxmox/common/step_finalize_config.go @@ -17,18 +17,18 @@ import ( // stepFinalizeTemplateConfig does any required modifications to the configuration _after_ // the VM has been converted into a template, such as updating name and description, or // unmounting the installation ISO. -type stepFinalizeTemplateConfig struct{} +type stepFinalizeConfig struct{} -type templateFinalizer interface { +type finalizer interface { GetVmConfig(*proxmox.VmRef) (map[string]interface{}, error) SetVmConfig(*proxmox.VmRef, map[string]interface{}) (interface{}, error) } -var _ templateFinalizer = &proxmox.Client{} +var _ finalizer = &proxmox.Client{} -func (s *stepFinalizeTemplateConfig) Run(ctx context.Context, state multistep.StateBag) multistep.StepAction { +func (s *stepFinalizeConfig) Run(ctx context.Context, state multistep.StateBag) multistep.StepAction { ui := state.Get("ui").(packersdk.Ui) - client := state.Get("proxmoxClient").(templateFinalizer) + client := state.Get("proxmoxClient").(finalizer) c := state.Get("config").(*Config) vmRef := state.Get("vmRef").(*proxmox.VmRef) @@ -151,4 +151,4 @@ func (s *stepFinalizeTemplateConfig) Run(ctx context.Context, state multistep.St return multistep.ActionContinue } -func (s *stepFinalizeTemplateConfig) Cleanup(state multistep.StateBag) {} +func (s *stepFinalizeConfig) Cleanup(state multistep.StateBag) {} diff --git a/builder/proxmox/common/step_finalize_template_config_test.go b/builder/proxmox/common/step_finalize_config_test.go similarity index 98% rename from builder/proxmox/common/step_finalize_template_config_test.go rename to builder/proxmox/common/step_finalize_config_test.go index 72223813..e74af929 100644 --- a/builder/proxmox/common/step_finalize_template_config_test.go +++ b/builder/proxmox/common/step_finalize_config_test.go @@ -27,7 +27,7 @@ func (m finalizerMock) SetVmConfig(vmref *proxmox.VmRef, c map[string]interface{ return m.setConfig(c) } -var _ templateFinalizer = finalizerMock{} +var _ finalizer = finalizerMock{} func TestTemplateFinalize(t *testing.T) { cs := []struct { @@ -218,7 +218,7 @@ func TestTemplateFinalize(t *testing.T) { state.Put("vmRef", proxmox.NewVmRef(1)) state.Put("proxmoxClient", finalizer) - step := stepFinalizeTemplateConfig{} + step := stepFinalizeConfig{} action := step.Run(context.TODO(), state) if action != c.expectedAction { t.Errorf("Expected action to be %v, got %v", c.expectedAction, action) diff --git a/builder/proxmox/iso/config.hcl2spec.go b/builder/proxmox/iso/config.hcl2spec.go index 3e77a919..a2a90f9e 100644 --- a/builder/proxmox/iso/config.hcl2spec.go +++ b/builder/proxmox/iso/config.hcl2spec.go @@ -114,6 +114,7 @@ type FlatConfig struct { DisableKVM *bool `mapstructure:"disable_kvm" cty:"disable_kvm" hcl:"disable_kvm"` TemplateName *string `mapstructure:"template_name" cty:"template_name" hcl:"template_name"` TemplateDescription *string `mapstructure:"template_description" cty:"template_description" hcl:"template_description"` + SkipConvertToTemplate *bool `mapstructure:"skip_convert_to_template" cty:"skip_convert_to_template" hcl:"skip_convert_to_template"` CloudInit *bool `mapstructure:"cloud_init" cty:"cloud_init" hcl:"cloud_init"` CloudInitStoragePool *string `mapstructure:"cloud_init_storage_pool" cty:"cloud_init_storage_pool" hcl:"cloud_init_storage_pool"` CloudInitDiskType *string `mapstructure:"cloud_init_disk_type" cty:"cloud_init_disk_type" hcl:"cloud_init_disk_type"` @@ -247,6 +248,7 @@ func (*FlatConfig) HCL2Spec() map[string]hcldec.Spec { "disable_kvm": &hcldec.AttrSpec{Name: "disable_kvm", Type: cty.Bool, Required: false}, "template_name": &hcldec.AttrSpec{Name: "template_name", Type: cty.String, Required: false}, "template_description": &hcldec.AttrSpec{Name: "template_description", Type: cty.String, Required: false}, + "skip_convert_to_template": &hcldec.AttrSpec{Name: "skip_convert_to_template", Type: cty.Bool, Required: false}, "cloud_init": &hcldec.AttrSpec{Name: "cloud_init", Type: cty.Bool, Required: false}, "cloud_init_storage_pool": &hcldec.AttrSpec{Name: "cloud_init_storage_pool", Type: cty.String, Required: false}, "cloud_init_disk_type": &hcldec.AttrSpec{Name: "cloud_init_disk_type", Type: cty.String, Required: false}, diff --git a/docs-partials/builder/proxmox/common/Config-not-required.mdx b/docs-partials/builder/proxmox/common/Config-not-required.mdx index bf8dea12..5e866da4 100644 --- a/docs-partials/builder/proxmox/common/Config-not-required.mdx +++ b/docs-partials/builder/proxmox/common/Config-not-required.mdx @@ -125,6 +125,9 @@ - `template_description` (string) - Description of the template, visible in the Proxmox interface. +- `skip_convert_to_template` (bool) - Skip converting the VM to a template on completion of build. + Defaults to `false` + - `cloud_init` (bool) - If true, add an empty Cloud-Init CDROM drive after the virtual machine has been converted to a template. Defaults to `false`.