Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Optimize Image upload to avoid buffering. Add cluster related fields in image upload resource & data source for PC #432

Merged
merged 9 commits into from
Apr 29, 2022
Merged

Optimize Image upload to avoid buffering. Add cluster related fields in image upload resource & data source for PC #432

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
071799a
Avoid buffering for upload request
bhatipradeep Apr 23, 2022
2ee32ea
Avoid buffering for NewUnAuthUploadRequest
bhatipradeep Apr 24, 2022
8c80ddd
Add content length in req for upload
bhatipradeep Apr 26, 2022
ce52d7c
Add body closer for upload code
bhatipradeep Apr 27, 2022
b05f928
Minor changes
bhatipradeep Apr 27, 2022
606f2cd
Add cluster reference attribute for image upload
bhatipradeep Apr 27, 2022
79799e8
Add cluster related fields in image resource and data sources
bhatipradeep Apr 28, 2022
0e58074
Update test for changes to image upload resource
bhatipradeep Apr 28, 2022
2c72c86
Remove name as attr for cluster ref in image
bhatipradeep Apr 29, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 32 additions & 10 deletions client/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"log"
"net/http"
"net/url"
"os"
"strings"

"github.com/PaesslerAG/jsonpath"
Expand All @@ -27,6 +28,7 @@ const (
// userAgent = "nutanix/" + libraryVersion
mediaType = "application/json"
formEncodedType = "application/x-www-form-urlencoded"
octetStreamType = "application/octet-stream"
)

// Client Config Configuration of the client
Expand Down Expand Up @@ -281,7 +283,7 @@ func (c *Client) NewUnAuthFormEncodedRequest(ctx context.Context, method, urlStr
}

// NewUploadRequest Handles image uploads for image service
func (c *Client) NewUploadRequest(ctx context.Context, method, urlStr string, body []byte) (*http.Request, error) {
func (c *Client) NewUploadRequest(ctx context.Context, method, urlStr string, fileReader *os.File) (*http.Request, error) {
// check if client exists or not
if c.client == nil {
return nil, fmt.Errorf(c.ErrorMsg)
Expand All @@ -293,16 +295,26 @@ func (c *Client) NewUploadRequest(ctx context.Context, method, urlStr string, bo

u := c.BaseURL.ResolveReference(rel)

buf := bytes.NewBuffer(body)
req, err := http.NewRequest(method, u.String(), fileReader)

req, err := http.NewRequest(method, u.String(), buf)
if err != nil {
return nil, err
}

// Set req.ContentLength and req.GetBody as internally there is no implementation of such for os.File type reader
// http.NewRequest() only sets this values for types - bytes.Buffer, bytes.Reader and strings.Reader
// Refer https://github.com/golang/go/blob/a0f77e56b7a7ecb92dca3e2afdd56ee773c2cb07/src/net/http/request.go#L896
fileInfo, err := fileReader.Stat()
if err != nil {
return nil, err
}
req.ContentLength = fileInfo.Size()
req.GetBody = func() (io.ReadCloser, error) {
return io.NopCloser(fileReader), nil
}

req.Header.Add("Content-Type", "application/octet-stream")
req.Header.Add("Accept", "application/octet-stream")
req.Header.Add("Content-Type", octetStreamType)
req.Header.Add("Accept", mediaType)
req.Header.Add("User-Agent", c.UserAgent)
req.Header.Add("Authorization", "Basic "+
base64.StdEncoding.EncodeToString([]byte(c.Credentials.Username+":"+c.Credentials.Password)))
Expand All @@ -311,7 +323,7 @@ func (c *Client) NewUploadRequest(ctx context.Context, method, urlStr string, bo
}

// NewUploadRequest handles image uploads for image service
func (c *Client) NewUnAuthUploadRequest(ctx context.Context, method, urlStr string, body []byte) (*http.Request, error) {
func (c *Client) NewUnAuthUploadRequest(ctx context.Context, method, urlStr string, fileReader *os.File) (*http.Request, error) {
// check if client exists or not
if c.client == nil {
return nil, fmt.Errorf(c.ErrorMsg)
Expand All @@ -323,16 +335,26 @@ func (c *Client) NewUnAuthUploadRequest(ctx context.Context, method, urlStr stri

u := c.BaseURL.ResolveReference(rel)

buf := bytes.NewBuffer(body)
req, err := http.NewRequest(method, u.String(), fileReader)

req, err := http.NewRequest(method, u.String(), buf)
if err != nil {
return nil, err
}

// Set req.ContentLength and req.GetBody as internally there is no implementation of such for os.File type reader
// http.NewRequest() only sets this values for types - bytes.Buffer, bytes.Reader and strings.Reader
// Refer https://github.com/golang/go/blob/a0f77e56b7a7ecb92dca3e2afdd56ee773c2cb07/src/net/http/request.go#L896
Copy link

@WWhitt WWhitt Apr 27, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: While this is true for those data types, it doesn't seem necessary when using an *os.File, see my example in the issue -- it uploaded perfectly fine without pre-emptively specifying content-length. That being said, I totally support not relying on default behavior here.

Edit: while I did get uploads without doing that, it wasn't working how I thought. ignore me. I agreed with explicitly setting it anyway

fileInfo, err := fileReader.Stat()
if err != nil {
return nil, err
}
req.ContentLength = fileInfo.Size()
req.GetBody = func() (io.ReadCloser, error) {
return io.NopCloser(fileReader), nil
}

req.Header.Add("Content-Type", "application/octet-stream")
req.Header.Add("Accept", "application/octet-stream")
req.Header.Add("Content-Type", octetStreamType)
req.Header.Add("Accept", mediaType)
req.Header.Add("User-Agent", c.UserAgent)
return req, nil
}
Expand Down
5 changes: 3 additions & 2 deletions client/client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"net/http"
"net/http/httptest"
"net/url"
"os"
"reflect"
"regexp"
"strings"
Expand Down Expand Up @@ -362,7 +363,7 @@ func TestClient_NewUploadRequest(t *testing.T) {
ctx context.Context
method string
urlStr string
body []byte
file *os.File
}

tests := []struct {
Expand All @@ -385,7 +386,7 @@ func TestClient_NewUploadRequest(t *testing.T) {
UserAgent: tt.fields.UserAgent,
onRequestCompleted: tt.fields.onRequestCompleted,
}
got, err := c.NewUploadRequest(tt.args.ctx, tt.args.method, tt.args.urlStr, tt.args.body)
got, err := c.NewUploadRequest(tt.args.ctx, tt.args.method, tt.args.urlStr, tt.args.file)
if (err != nil) != tt.wantErr {
t.Errorf("Client.NewUploadRequest() error = %v, wantErr %v", err, tt.wantErr)

Expand Down
9 changes: 1 addition & 8 deletions client/foundation/foundation_file_management_service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package foundation
import (
"context"
"fmt"
"io/ioutil"
"net/http"
"os"

Expand Down Expand Up @@ -56,13 +55,7 @@ func (fmo FileManagementOperations) UploadImage(ctx context.Context, installerTy
}
defer file.Close()

// read file as slice of bytes
fileContents, err := ioutil.ReadAll(file)
if err != nil {
return nil, fmt.Errorf("error while reading file: %s", err)
}

req, err := fmo.client.NewUnAuthUploadRequest(ctx, http.MethodPost, path, fileContents)
req, err := fmo.client.NewUnAuthUploadRequest(ctx, http.MethodPost, path, file)
if err != nil {
return nil, err
}
Expand Down
8 changes: 1 addition & 7 deletions client/v3/v3_service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package v3
import (
"context"
"fmt"
"io/ioutil"
"log"
"net/http"
"os"
Expand Down Expand Up @@ -367,12 +366,7 @@ func (op Operations) UploadImage(uuid, filepath string) error {
}
defer file.Close()

fileContents, err := ioutil.ReadAll(file)
if err != nil {
return fmt.Errorf("error: Cannot read file %s", err)
}

req, err := op.client.NewUploadRequest(ctx, http.MethodPut, path, fileContents)
req, err := op.client.NewUploadRequest(ctx, http.MethodPut, path, file)

if err != nil {
return fmt.Errorf("error: Creating request %s", err)
Expand Down
9 changes: 9 additions & 0 deletions client/v3/v3_structs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -885,6 +885,9 @@ type ImageResources struct {
// The image version
Version *ImageVersionResources `json:"version,omitempty" mapstructure:"version,omitempty"`

// Cluster reference lists
InitialPlacementRefList []*ReferenceValues `json:"initial_placement_ref_list,omitempty" mapstructure:"initial_placement_ref_list, omitempty"`

// Reference to the source image such as 'vm_disk
DataSourceReference *Reference `json:"data_source_reference,omitempty" mapstructure:"data_source_reference,omitempty"`
}
Expand Down Expand Up @@ -990,6 +993,12 @@ type ImageResourcesDefStatus struct {
// The source URI points at the location of a the source image which is used to create/update image.
SourceURI *string `json:"source_uri,omitempty" mapstructure:"source_uri,omitempty"`

// Cluster reference lists
InitialPlacementRefList []*ReferenceValues `json:"initial_placement_ref_list,omitempty" mapstructure:"initial_placement_ref_list, omitempty"`

// cluster reference list when request was made without refs
CurrentClusterReferenceList []*ReferenceValues `json:"current_cluster_reference_list,omitempty" mapstructure:"current_cluster_reference_list, omitempty"`

// The image version
Version *ImageVersionStatus `json:"version,omitempty" mapstructure:"version,omitempty"`
}
Expand Down
89 changes: 89 additions & 0 deletions nutanix/data_source_nutanix_image.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,46 @@ func dataSourceNutanixImage() *schema.Resource {
Computed: true,
Elem: &schema.Schema{Type: schema.TypeString},
},
"cluster_references": {
Type: schema.TypeList,
Computed: true,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"kind": {
Type: schema.TypeString,
Computed: true,
},
"uuid": {
Type: schema.TypeString,
Computed: true,
},
"name": {
Type: schema.TypeString,
Computed: true,
},
},
},
},
"current_cluster_reference_list": {
Type: schema.TypeList,
Computed: true,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"kind": {
Type: schema.TypeString,
Computed: true,
},
"uuid": {
Type: schema.TypeString,
Computed: true,
},
"name": {
Type: schema.TypeString,
Computed: true,
},
},
},
},
"image_type": {
Type: schema.TypeString,
Computed: true,
Expand Down Expand Up @@ -165,6 +205,15 @@ func dataSourceNutanixImageRead(ctx context.Context, d *schema.ResourceData, met
if err := d.Set("availability_zone_reference", flattenReferenceValues(resp.Status.AvailabilityZoneReference)); err != nil {
return diag.FromErr(err)
}

if err := d.Set("cluster_references", flattenArrayOfReferenceValues(resp.Status.Resources.InitialPlacementRefList)); err != nil {
return diag.FromErr(err)
}

if err := d.Set("current_cluster_reference_list", flattenArrayOfReferenceValues(resp.Status.Resources.CurrentClusterReferenceList)); err != nil {
return diag.FromErr(err)
}

if err := flattenClusterReference(resp.Status.ClusterReference, d); err != nil {
return diag.FromErr(err)
}
Expand Down Expand Up @@ -301,6 +350,46 @@ func resourceNutanixDatasourceImageInstanceResourceV0() *schema.Resource {
Computed: true,
Elem: &schema.Schema{Type: schema.TypeString},
},
"cluster_references": {
Type: schema.TypeList,
Computed: true,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"kind": {
Type: schema.TypeString,
Computed: true,
},
"uuid": {
Type: schema.TypeString,
Computed: true,
},
"name": {
Type: schema.TypeString,
Computed: true,
},
},
},
},
"current_cluster_reference_list": {
Type: schema.TypeList,
Computed: true,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"kind": {
Type: schema.TypeString,
Computed: true,
},
"uuid": {
Type: schema.TypeString,
Computed: true,
},
"name": {
Type: schema.TypeString,
Computed: true,
},
},
},
},
"image_type": {
Type: schema.TypeString,
Computed: true,
Expand Down
42 changes: 42 additions & 0 deletions nutanix/helpers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,31 @@ func validateArrayRef(references interface{}, kindValue *string) []*v3.Reference
return nil
}

func validateArrayRefValues(references interface{}, kindValue string) []*v3.ReferenceValues {
refs := make([]*v3.ReferenceValues, 0)

for _, s := range references.([]interface{}) {
ref := s.(map[string]interface{})
r := v3.ReferenceValues{}

if kindValue != "" {
r.Kind = kindValue
} else {
r.Kind = ref["kind"].(string)
}

r.UUID = ref["uuid"].(string)
r.Name = ref["name"].(string)

refs = append(refs, &r)
}
if len(refs) > 0 {
return refs
}

return nil
}

func flattenArrayReferenceValues(refs []*v3.Reference) []map[string]interface{} {
references := make([]map[string]interface{}, 0)
for _, r := range refs {
Expand Down Expand Up @@ -329,3 +354,20 @@ func flattenReferenceValuesList(r *v3.Reference) []interface{} {
}
return references
}

func flattenArrayOfReferenceValues(refs []*v3.ReferenceValues) []map[string]interface{} {
references := make([]map[string]interface{}, 0)
for _, r := range refs {
reference := make(map[string]interface{})
if r != nil {
reference["kind"] = r.Kind
reference["uuid"] = r.UUID

if r.Name != "" {
reference["name"] = r.Name
}
references = append(references, reference)
}
}
return references
}
Loading