From 94018c691cced9f004c4ec8b9004c1b5ac7b8e5e Mon Sep 17 00:00:00 2001 From: tcarrio Date: Sun, 15 Jul 2018 23:35:02 -0400 Subject: [PATCH] Fixed step interface, added step to builder, passing all tests for new image_query.go --- builder/openstack/builder.go | 5 + builder/openstack/image_query.go | 123 ++++++++++++++++---- builder/openstack/image_query_test.go | 82 +++++++++---- builder/openstack/run_config.go | 35 +++--- builder/openstack/step_source_image_info.go | 33 +++--- 5 files changed, 200 insertions(+), 78 deletions(-) diff --git a/builder/openstack/builder.go b/builder/openstack/builder.go index 2938d67c0..a2b34ddfa 100644 --- a/builder/openstack/builder.go +++ b/builder/openstack/builder.go @@ -86,6 +86,11 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe PrivateKeyFile: b.config.RunConfig.Comm.SSHPrivateKey, SSHAgentAuth: b.config.RunConfig.Comm.SSHAgentAuth, }, + &StepSourceImageInfo{ + SourceImage: b.config.SourceImage, + SourceImageName: b.config.SourceImageName, + ImageFilters: b.config.SourceImageFilters, + }, &StepCreateVolume{ UseBlockStorageVolume: b.config.UseBlockStorageVolume, SourceImage: b.config.SourceImage, diff --git a/builder/openstack/image_query.go b/builder/openstack/image_query.go index b86599801..2eb54eeb3 100644 --- a/builder/openstack/image_query.go +++ b/builder/openstack/image_query.go @@ -3,13 +3,13 @@ package openstack import ( "fmt" "reflect" + "strconv" "strings" "time" - "strconv" - "github.com/hashicorp/packer/packer" - "github.com/hashicorp/go-multierror" "github.com/gophercloud/gophercloud/openstack/imageservice/v2/images" + "github.com/hashicorp/go-multierror" + "github.com/hashicorp/packer/packer" ) const ( @@ -35,7 +35,47 @@ func getDateFilter(s string) (images.ImageDateFilter, error) { } var badFilter images.ImageDateFilter - return badFilter, fmt.Errorf("No ImageDateFilter found for %s", s) + return badFilter, fmt.Errorf("No valid ImageDateFilter found for %s", s) +} + +// Retrieve the specific ImageVisibility using the exported const from images +func getImageVisibility(s string) (images.ImageVisibility, error) { + visibilities := [...]images.ImageVisibility{ + images.ImageVisibilityPublic, + images.ImageVisibilityPrivate, + images.ImageVisibilityCommunity, + images.ImageVisibilityShared, + } + + for _, visibility := range visibilities { + if string(visibility) == s { + return visibility, nil + } + } + + var nilVisibility images.ImageVisibility + return nilVisibility, fmt.Errorf("No valid ImageVisilibility found for %s", s) +} + +// Retrieve the specific ImageVisibility using the exported const from images +func getImageStatus(s string) (images.ImageStatus, error) { + statuses := [...]images.ImageStatus{ + images.ImageStatusActive, + images.ImageStatusDeactivated, + images.ImageStatusDeleted, + images.ImageStatusPendingDelete, + images.ImageStatusQueued, + images.ImageStatusSaving, + } + + for _, status := range statuses { + if string(status) == s { + return status, nil + } + } + + var nilStatus images.ImageStatus + return nilStatus, fmt.Errorf("No valid ImageVisilibility found for %s", s) } // Allows construction of all fields from ListOpts using the "q" tags and @@ -43,7 +83,7 @@ func getDateFilter(s string) (images.ImageDateFilter, error) { func buildImageFilters(input map[string]string, listOpts *images.ListOpts) *packer.MultiError { // fill each field in the ListOpts based on tag/type - metaOpts := reflect.ValueOf(listOpts).Elem() + metaOpts := reflect.Indirect(reflect.ValueOf(listOpts)) multiErr := packer.MultiError{} @@ -57,17 +97,46 @@ func buildImageFilters(input map[string]string, listOpts *images.ListOpts) *pack if val, exists := input[key]; exists && vField.CanSet() { switch vField.Kind() { - case reflect.Int64: + // Handles integer types used in ListOpts + case reflect.Int64, reflect.Int: iVal, err := strconv.Atoi(val) if err != nil { multierror.Append(err, multiErr.Errors...) - } else { - vField.Set(reflect.ValueOf(iVal)) + continue } - case reflect.String: - vField.Set(reflect.ValueOf(val)) + if vField.Kind() == reflect.Int { + vField.Set(reflect.ValueOf(iVal)) + } else { + var i64Val int64 + i64Val = int64(iVal) + vField.Set(reflect.ValueOf(i64Val)) + } + // Handles string and types using string + case reflect.String: + switch vField.Type() { + default: + vField.Set(reflect.ValueOf(val)) + + case reflect.TypeOf(images.ImageVisibility("")): + iv, err := getImageVisibility(val) + if err != nil { + multierror.Append(err, multiErr.Errors...) + continue + } + vField.Set(reflect.ValueOf(iv)) + + case reflect.TypeOf(images.ImageStatus("")): + is, err := getImageStatus(val) + if err != nil { + multierror.Append(err, multiErr.Errors...) + continue + } + vField.Set(reflect.ValueOf(is)) + } + + // Generates slice of strings for Tags case reflect.Slice: typeOfSlice := reflect.TypeOf(vField).Elem() fieldArray := reflect.MakeSlice(reflect.SliceOf(typeOfSlice), 0, 0) @@ -80,18 +149,21 @@ func buildImageFilters(input map[string]string, listOpts *images.ListOpts) *pack default: multierror.Append( - fmt.Errorf("Unsupported struct type %s", vField.Type().Name), + fmt.Errorf("Unsupported kind %s", vField.Kind()), multiErr.Errors...) } - } else if fieldName == reflect.TypeOf(images.ListOpts{}.CreatedAtQuery).Name() || - fieldName == reflect.TypeOf(images.ListOpts{}.UpdatedAtQuery).Name() { + // Handles ImageDateQuery types + } else if fieldName == reflect.TypeOf(listOpts.CreatedAtQuery).Name() || + fieldName == reflect.TypeOf(listOpts.UpdatedAtQuery).Name() { + // get ImageDateQuery from string and set to this field - query, err := dateToImageDateQuery(&key, &val) + query, err := dateToImageDateQuery(key, val) if err != nil { multierror.Append(err, multiErr.Errors...) continue } + vField.Set(reflect.ValueOf(query)) } } @@ -104,13 +176,12 @@ func buildImageFilters(input map[string]string, listOpts *images.ListOpts) *pack // It is suggested that users use the newest sort field // See https://developer.openstack.org/api-ref/image/v2/ func applyMostRecent(listOpts *images.ListOpts) { - // apply to old sorting properties if user used them. This overwrites previous values? - if listOpts.SortDir == "" && listOpts.SortKey != "" { - listOpts.SortDir = descendingSort - listOpts.SortKey = createdAtKey - } + // Apply to old sorting properties if user used them. This overwrites previous values. + // The docs don't seem to mention more than one field being allowed here and how they would be + listOpts.SortDir = descendingSort + listOpts.SortKey = createdAtKey - // apply to new sorting property + // Apply to new sorting property. if listOpts.Sort != "" { listOpts.Sort = fmt.Sprintf("%s:%s,%s", createdAtKey, descendingSort, listOpts.Sort) } else { @@ -121,10 +192,10 @@ func applyMostRecent(listOpts *images.ListOpts) { } // Converts a given date entry to ImageDateQuery for use in ListOpts -func dateToImageDateQuery(val *string, key *string) (*images.ImageDateQuery, error) { +func dateToImageDateQuery(val string, key string) (*images.ImageDateQuery, error) { q := new(images.ImageDateQuery) sep := ":" - entries := strings.Split(*val, sep) + entries := strings.Split(val, sep) if len(entries) > 3 { filter, err := getDateFilter(entries[0]) @@ -134,9 +205,13 @@ func dateToImageDateQuery(val *string, key *string) (*images.ImageDateQuery, err q.Filter = filter } - date, err := time.Parse((*val)[len(entries[0]):], time.RFC3339) + dateSubstr := val[len(entries[0])+1:] + date, err := time.Parse(time.RFC3339, dateSubstr) if err != nil { - return nil, fmt.Errorf("Failed to parse date format for %s", key) + return nil, fmt.Errorf("Failed to parse date format for %s.\nDate: %s.\nError: %s", + key, + dateSubstr, + err.Error()) } else { q.Date = date } diff --git a/builder/openstack/image_query_test.go b/builder/openstack/image_query_test.go index 32f605a91..0876ccf09 100644 --- a/builder/openstack/image_query_test.go +++ b/builder/openstack/image_query_test.go @@ -2,17 +2,19 @@ package openstack import ( "testing" + "github.com/gophercloud/gophercloud/openstack/imageservice/v2/images" + "github.com/mitchellh/mapstructure" ) func TestGetImageFilter(t *testing.T) { passedExpectedMap := map[string]images.ImageDateFilter{ - "gt": images.FilterGT, - "gte": images.FilterGTE, - "lt": images.FilterLT, - "lte": images.FilterLTE, - "neq": images.FilterNEQ, - "eq": images.FilterEQ, + "gt": images.FilterGT, + "gte": images.FilterGTE, + "lt": images.FilterLT, + "lte": images.FilterLTE, + "neq": images.FilterNEQ, + "eq": images.FilterEQ, } for passed, expected := range passedExpectedMap { @@ -29,12 +31,12 @@ func TestBuildImageFilter(t *testing.T) { testOpts := images.ListOpts{} filters := map[string]string{ - "limit": "3", - "name": "Ubuntu 16.04", + "limit": "3", + "name": "Ubuntu 16.04", "visibility": "public", - "image_status": "active", - "size_min": "0", - "sort": "created_at:desc", + "status": "active", + "size_min": "0", + "sort": "created_at:desc", } multiErr := buildImageFilters(filters, &testOpts) @@ -46,11 +48,11 @@ func TestBuildImageFilter(t *testing.T) { } if testOpts.Limit != 3 { - t.Errorf("Limit did not parse correctly") + t.Errorf("Limit did not parse correctly: %d", testOpts.Limit) } if testOpts.Name != filters["name"] { - t.Errorf("Name did not parse correctly") + t.Errorf("Name did not parse correctly: %") } var visibility images.ImageVisibility = "public" @@ -60,7 +62,7 @@ func TestBuildImageFilter(t *testing.T) { var imageStatus images.ImageStatus = "active" if testOpts.Status != imageStatus { - t.Errorf("Image status did not parse correctly") + t.Errorf("Image status did not parse correctly: %s", testOpts.Status) } if testOpts.SizeMin != 0 { @@ -73,30 +75,64 @@ func TestBuildImageFilter(t *testing.T) { } func TestApplyMostRecent(t *testing.T) { - testOpts := images.ListOpts{ - Name: "RHEL 7.0", + testSortEmptyOpts := images.ListOpts{ + Name: "RHEL 7.0", SizeMin: 0, } - applyMostRecent(&testOpts) + testSortFilledOpts := images.ListOpts{ + Name: "Ubuntu 16.04", + SizeMin: 0, + Sort: "tags:ubuntu", + } - if testOpts.Sort != "created_at:desc" { + applyMostRecent(&testSortEmptyOpts) + + if testSortEmptyOpts.Sort != "created_at:desc" { t.Errorf("Error applying most recent filter: sort") } - if testOpts.SortDir != "desc" || testOpts.SortKey != "created_at" { - t.Errorf("Error applying most recent filter: sort_dir/sort_key") + if testSortEmptyOpts.SortDir != "desc" || testSortEmptyOpts.SortKey != "created_at" { + t.Errorf("Error applying most recent filter: sort_dir/sort_key:\n{sort_dir: %s, sort_key: %s}", + testSortEmptyOpts.SortDir, testSortEmptyOpts.SortKey) + } + + applyMostRecent(&testSortFilledOpts) + + if testSortFilledOpts.Sort != "created_at:desc,tags:ubuntu" { + t.Errorf("Error applying most recent filter: sort") + } + + if testSortFilledOpts.SortDir != "desc" || testSortFilledOpts.SortKey != "created_at" { + t.Errorf("Error applying most recent filter: sort_dir/sort_key:\n{sort_dir: %s, sort_key: %s}", + testSortFilledOpts.SortDir, testSortFilledOpts.SortKey) } } func TestDateToImageDateQuery(t *testing.T) { tests := [][2]string{ - {"2006-01-02T15:04:05Z07:00", "created_at"}, + {"gt:2012-11-01T22:08:41+00:00", "created_at"}, } for _, test := range tests { - if _, err := dateToImageDateQuery(&test[0], &test[1]); err != nil { + if _, err := dateToImageDateQuery(test[0], test[1]); err != nil { t.Error(err) } } -} \ No newline at end of file +} + +func TestImageFilterOptionsDecode(t *testing.T) { + opts := ImageFilterOptions{} + input := map[string]interface{}{ + "most_recent": true, + "filters": map[string]interface{}{ + "visibility": "protected", + "tag": "prod", + "name": "ubuntu 16.04", + }, + } + err := mapstructure.Decode(input, &opts) + if err != nil { + t.Error("Did not successfully generate ImageFilterOptions from %v. Contains %v", input, opts) + } +} diff --git a/builder/openstack/run_config.go b/builder/openstack/run_config.go index c96eb46bb..0db1110bd 100644 --- a/builder/openstack/run_config.go +++ b/builder/openstack/run_config.go @@ -18,21 +18,22 @@ type RunConfig struct { SSHInterface string `mapstructure:"ssh_interface"` SSHIPVersion string `mapstructure:"ssh_ip_version"` - SourceImage string `mapstructure:"source_image"` - SourceImageName string `mapstructure:"source_image_name"` - Flavor string `mapstructure:"flavor"` - AvailabilityZone string `mapstructure:"availability_zone"` - RackconnectWait bool `mapstructure:"rackconnect_wait"` - FloatingIPNetwork string `mapstructure:"floating_ip_network"` - FloatingIP string `mapstructure:"floating_ip"` - ReuseIPs bool `mapstructure:"reuse_ips"` - SecurityGroups []string `mapstructure:"security_groups"` - Networks []string `mapstructure:"networks"` - Ports []string `mapstructure:"ports"` - UserData string `mapstructure:"user_data"` - UserDataFile string `mapstructure:"user_data_file"` - InstanceName string `mapstructure:"instance_name"` - InstanceMetadata map[string]string `mapstructure:"instance_metadata"` + SourceImage string `mapstructure:"source_image"` + SourceImageName string `mapstructure:"source_image_name"` + SourceImageFilters ImageFilterOptions `mapstructure:"source_image_filter"` + Flavor string `mapstructure:"flavor"` + AvailabilityZone string `mapstructure:"availability_zone"` + RackconnectWait bool `mapstructure:"rackconnect_wait"` + FloatingIPNetwork string `mapstructure:"floating_ip_network"` + FloatingIP string `mapstructure:"floating_ip"` + ReuseIPs bool `mapstructure:"reuse_ips"` + SecurityGroups []string `mapstructure:"security_groups"` + Networks []string `mapstructure:"networks"` + Ports []string `mapstructure:"ports"` + UserData string `mapstructure:"user_data"` + UserDataFile string `mapstructure:"user_data_file"` + InstanceName string `mapstructure:"instance_name"` + InstanceMetadata map[string]string `mapstructure:"instance_metadata"` ConfigDrive bool `mapstructure:"config_drive"` @@ -75,8 +76,8 @@ func (c *RunConfig) Prepare(ctx *interpolate.Context) []error { } } - if c.SourceImage == "" && c.SourceImageName == "" { - errs = append(errs, errors.New("Either a source_image or a source_image_`name must be specified")) + if c.SourceImage == "" && c.SourceImageName == "" && c.SourceImageFilters.Filters != nil { + errs = append(errs, errors.New("Either a source_image, a source_image_name, or must be specified")) } else if len(c.SourceImage) > 0 && len(c.SourceImageName) > 0 { errs = append(errs, errors.New("Only a source_image or a source_image_name can be specified, not both.")) } diff --git a/builder/openstack/step_source_image_info.go b/builder/openstack/step_source_image_info.go index c3bf4486d..116bdfa67 100644 --- a/builder/openstack/step_source_image_info.go +++ b/builder/openstack/step_source_image_info.go @@ -1,26 +1,25 @@ package openstack import ( - "log" - "fmt" "context" + "fmt" + "log" - - "github.com/hashicorp/packer/packer" - "github.com/hashicorp/packer/helper/multistep" "github.com/gophercloud/gophercloud/openstack/imageservice/v2/images" "github.com/gophercloud/gophercloud/pagination" + "github.com/hashicorp/packer/helper/multistep" + "github.com/hashicorp/packer/packer" ) type StepSourceImageInfo struct { - SourceImage string - SourceImageName string - ImageFilters ImageFilterOptions + SourceImage string + SourceImageName string + ImageFilters ImageFilterOptions } type ImageFilterOptions struct { - Filters map[string]string - MostRecent bool `mapstructure:"most_recent"` + Filters map[string]string `mapstructure:"filters"` + MostRecent bool `mapstructure:"most_recent"` } func (s *StepSourceImageInfo) Run(_ context.Context, state multistep.StateBag) multistep.StepAction { @@ -54,7 +53,7 @@ func (s *StepSourceImageInfo) Run(_ context.Context, state multistep.StateBag) m log.Printf("Using Image Filters %v", params) image := &images.Image{} - err = images.List(client, params).EachPage(func (page pagination.Page) (bool, error) { + err = images.List(client, params).EachPage(func(page pagination.Page) (bool, error) { i, err := images.ExtractImages(page) if err != nil { return false, err @@ -62,12 +61,14 @@ func (s *StepSourceImageInfo) Run(_ context.Context, state multistep.StateBag) m switch len(i) { case 0: - return false, fmt.Errorf("No image was found matching filters: %v", ) + return false, fmt.Errorf("No image was found matching filters: %v", params) case 1: *image = i[0] return true, nil default: - return false, fmt.Errorf("Your query returned more than one result. Please try a more specific search, or set most_recent to true.") + return false, fmt.Errorf( + "Your query returned more than one result. Please try a more specific search, or set most_recent to true. Search filters: %v", + params) } return true, nil @@ -84,4 +85,8 @@ func (s *StepSourceImageInfo) Run(_ context.Context, state multistep.StateBag) m state.Put("source_image", image) return multistep.ActionContinue -} \ No newline at end of file +} + +func (s *StepSourceImageInfo) Cleanup(state multistep.StateBag) { + // No cleanup required for backout +}