Skip to content

Commit

Permalink
Use trace.NewOperation pattern in vic-machine service API handlers (#…
Browse files Browse the repository at this point in the history
…6563)

* trace.NewOperation pattern used for vic-machine service API

* whitespace

* whitespace

* clean up trace.op pattern in API handler functions

* bug fix: validateTarget takes in context since used by buildData
  • Loading branch information
AngieCris authored and zjs committed Nov 20, 2017
1 parent d30bb16 commit b4bacb0
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 52 deletions.
4 changes: 2 additions & 2 deletions lib/apiservers/service/restapi/handlers/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,12 +94,12 @@ func validateTarget(ctx context.Context, d *data.Data) (*validate.Validator, err
}

// Copied from list.go, and appears to be present other places. TODO: deduplicate
func upgradeStatusMessage(ctx context.Context, vch *vm.VirtualMachine, installerVer *version.Build, vchVer *version.Build) string {
func upgradeStatusMessage(op trace.Operation, vch *vm.VirtualMachine, installerVer *version.Build, vchVer *version.Build) string {
if sameVer := installerVer.Equal(vchVer); sameVer {
return "Up to date"
}

upgrading, err := vch.VCHUpdateStatus(ctx)
upgrading, err := vch.VCHUpdateStatus(op)
if err != nil {
return fmt.Sprintf("Unknown: %s", err)
}
Expand Down
36 changes: 19 additions & 17 deletions lib/apiservers/service/restapi/handlers/vch_create.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
package handlers

import (
"context"
"fmt"
"io"
"math"
Expand Down Expand Up @@ -44,6 +43,7 @@ import (
"github.com/vmware/vic/lib/install/vchlog"
"github.com/vmware/vic/pkg/ip"
viclog "github.com/vmware/vic/pkg/log"
"github.com/vmware/vic/pkg/trace"
"github.com/vmware/vic/pkg/version"
)

Expand Down Expand Up @@ -77,17 +77,18 @@ func (h *VCHCreate) Handle(params operations.PostTargetTargetVchParams, principa
return operations.NewPostTargetTargetVchDefault(util.StatusCode(err)).WithPayload(&models.Error{Message: err.Error()})
}

validator, err := validateTarget(params.HTTPRequest.Context(), d)
op := trace.NewOperation(params.HTTPRequest.Context(), "vch create handler")
validator, err := validateTarget(op, d)
if err != nil {
return operations.NewPostTargetTargetVchDefault(400).WithPayload(&models.Error{Message: err.Error()})
}

c, err := buildCreate(params.HTTPRequest.Context(), d, validator.Session.Finder, params.Vch)
c, err := buildCreate(op, d, validator.Session.Finder, params.Vch)
if err != nil {
return operations.NewPostTargetTargetVchDefault(util.StatusCode(err)).WithPayload(&models.Error{Message: err.Error()})
}

task, err := handleCreate(params.HTTPRequest.Context(), c, validator)
task, err := handleCreate(op, c, validator)
if err != nil {
return operations.NewPostTargetTargetVchDefault(util.StatusCode(err)).WithPayload(&models.Error{Message: err.Error()})
}
Expand All @@ -113,17 +114,18 @@ func (h *VCHDatacenterCreate) Handle(params operations.PostTargetTargetDatacente
return operations.NewPostTargetTargetDatacenterDatacenterVchDefault(util.StatusCode(err)).WithPayload(&models.Error{Message: err.Error()})
}

validator, err := validateTarget(params.HTTPRequest.Context(), d)
op := trace.NewOperation(params.HTTPRequest.Context(), "vch create handler")
validator, err := validateTarget(op, d)
if err != nil {
return operations.NewPostTargetTargetDatacenterDatacenterVchDefault(400).WithPayload(&models.Error{Message: err.Error()})
}

c, err := buildCreate(params.HTTPRequest.Context(), d, validator.Session.Finder, params.Vch)
c, err := buildCreate(op, d, validator.Session.Finder, params.Vch)
if err != nil {
return operations.NewPostTargetTargetDatacenterDatacenterVchDefault(util.StatusCode(err)).WithPayload(&models.Error{Message: err.Error()})
}

task, err := handleCreate(params.HTTPRequest.Context(), c, validator)
task, err := handleCreate(op, c, validator)
if err != nil {
return operations.NewPostTargetTargetDatacenterDatacenterVchDefault(util.StatusCode(err)).WithPayload(&models.Error{Message: err.Error()})
}
Expand Down Expand Up @@ -157,7 +159,7 @@ func setUpLogger() *os.File {
return localLogFile
}

func buildCreate(ctx context.Context, d *data.Data, finder *find.Finder, vch *models.VCH) (*create.Create, error) {
func buildCreate(op trace.Operation, d *data.Data, finder *find.Finder, vch *models.VCH) (*create.Create, error) {
c := &create.Create{Data: d}

// TODO: deduplicate with create.processParams
Expand Down Expand Up @@ -193,7 +195,7 @@ func buildCreate(ctx context.Context, d *data.Data, finder *find.Finder, vch *mo
c.ResourceLimits.VCHMemoryShares = fromShares(vch.Compute.Memory.Shares)
}

resourcePath, err := fromManagedObject(ctx, finder, "ResourcePool", vch.Compute.Resource) // TODO: Do we need to handle clusters differently?
resourcePath, err := fromManagedObject(op, finder, "ResourcePool", vch.Compute.Resource) // TODO: Do we need to handle clusters differently?
if err != nil {
return nil, util.NewError(400, fmt.Sprintf("Error finding resource pool: %s", err))
}
Expand All @@ -205,7 +207,7 @@ func buildCreate(ctx context.Context, d *data.Data, finder *find.Finder, vch *mo

if vch.Network != nil {
if vch.Network.Bridge != nil {
path, err := fromManagedObject(ctx, finder, "Network", vch.Network.Bridge.PortGroup)
path, err := fromManagedObject(op, finder, "Network", vch.Network.Bridge.PortGroup)
if err != nil {
return nil, util.NewError(400, fmt.Sprintf("Error finding bridge network: %s", err))
}
Expand All @@ -221,7 +223,7 @@ func buildCreate(ctx context.Context, d *data.Data, finder *find.Finder, vch *mo
}

if vch.Network.Client != nil {
path, err := fromManagedObject(ctx, finder, "Network", vch.Network.Client.PortGroup)
path, err := fromManagedObject(op, finder, "Network", vch.Network.Client.PortGroup)
if err != nil {
return nil, util.NewError(400, fmt.Sprintf("Error finding client network portgroup: %s", err))
}
Expand All @@ -238,7 +240,7 @@ func buildCreate(ctx context.Context, d *data.Data, finder *find.Finder, vch *mo
}

if vch.Network.Management != nil {
path, err := fromManagedObject(ctx, finder, "Network", vch.Network.Management.PortGroup)
path, err := fromManagedObject(op, finder, "Network", vch.Network.Management.PortGroup)
if err != nil {
return nil, util.NewError(400, fmt.Sprintf("Error finding management network portgroup: %s", err))
}
Expand All @@ -255,7 +257,7 @@ func buildCreate(ctx context.Context, d *data.Data, finder *find.Finder, vch *mo
}

if vch.Network.Public != nil {
path, err := fromManagedObject(ctx, finder, "Network", vch.Network.Public.PortGroup)
path, err := fromManagedObject(op, finder, "Network", vch.Network.Public.PortGroup)
if err != nil {
return nil, util.NewError(400, fmt.Sprintf("Error finding public network portgroup: %s", err))
}
Expand All @@ -282,7 +284,7 @@ func buildCreate(ctx context.Context, d *data.Data, finder *find.Finder, vch *mo
for _, cnetwork := range vch.Network.Container {
alias := cnetwork.Alias

path, err := fromManagedObject(ctx, finder, "Network", cnetwork.PortGroup)
path, err := fromManagedObject(op, finder, "Network", cnetwork.PortGroup)
if err != nil {
return nil, util.NewError(400, fmt.Sprintf("Error finding portgroup for container network %s: %s", alias, err))
}
Expand Down Expand Up @@ -421,7 +423,7 @@ func buildCreate(ctx context.Context, d *data.Data, finder *find.Finder, vch *mo
return c, nil
}

func handleCreate(ctx context.Context, c *create.Create, validator *validate.Validator) (*strfmt.URI, error) {
func handleCreate(op trace.Operation, c *create.Create, validator *validate.Validator) (*strfmt.URI, error) {
vchConfig, err := validator.Validate(validator.Context, c.Data)
vConfig := validator.AddDeprecatedFields(validator.Context, vchConfig, c.Data)

Expand Down Expand Up @@ -461,10 +463,10 @@ func fromNetworkAddress(m *models.NetworkAddress) string {
return string(m.Hostname)
}

func fromManagedObject(ctx context.Context, finder *find.Finder, t string, m *models.ManagedObject) (string, error) {
func fromManagedObject(op trace.Operation, finder *find.Finder, t string, m *models.ManagedObject) (string, error) {
if m.ID != "" {
managedObjectReference := types.ManagedObjectReference{Type: t, Value: m.ID}
element, err := finder.Element(ctx, managedObjectReference)
element, err := finder.Element(op, managedObjectReference)

if err != nil {
return "", err
Expand Down
22 changes: 11 additions & 11 deletions lib/apiservers/service/restapi/handlers/vch_get.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ package handlers

import (
"bytes"
"context"
"encoding/pem"
"fmt"
"net"
Expand All @@ -37,6 +36,7 @@ import (
"github.com/vmware/vic/lib/install/data"
"github.com/vmware/vic/lib/install/management"
"github.com/vmware/vic/lib/install/validate"
"github.com/vmware/vic/pkg/trace"
"github.com/vmware/vic/pkg/version"
"github.com/vmware/vic/pkg/vsphere/vm"
)
Expand All @@ -63,8 +63,8 @@ func (h *VCHGet) Handle(params operations.GetTargetTargetVchVchIDParams, princip

d.ID = params.VchID

vch, err := getVCH(params.HTTPRequest.Context(), d)

op := trace.NewOperation(params.HTTPRequest.Context(), "vch: %s", params.VchID)
vch, err := getVCH(op, d)
if err != nil {
return operations.NewGetTargetTargetVchVchIDDefault(util.StatusCode(err)).WithPayload(&models.Error{Message: err.Error()})
}
Expand All @@ -86,17 +86,17 @@ func (h *VCHDatacenterGet) Handle(params operations.GetTargetTargetDatacenterDat

d.ID = params.VchID

vch, err := getVCH(params.HTTPRequest.Context(), d)

op := trace.NewOperation(params.HTTPRequest.Context(), "vch: %s", params.VchID)
vch, err := getVCH(op, d)
if err != nil {
return operations.NewGetTargetTargetDatacenterDatacenterVchVchIDDefault(util.StatusCode(err)).WithPayload(&models.Error{Message: err.Error()})
}

return operations.NewGetTargetTargetDatacenterDatacenterVchVchIDOK().WithPayload(vch)
}

func getVCH(ctx context.Context, d *data.Data) (*models.VCH, error) {
validator, err := validateTarget(ctx, d)
func getVCH(op trace.Operation, d *data.Data) (*models.VCH, error) {
validator, err := validateTarget(op, d)
if err != nil {
return nil, util.WrapError(400, err)
}
Expand All @@ -112,15 +112,15 @@ func getVCH(ctx context.Context, d *data.Data) (*models.VCH, error) {
return nil, util.NewError(500, fmt.Sprintf("Failed to load VCH data: %s", err))
}

model, err := vchToModel(vch, d, executor)
model, err := vchToModel(op, vch, d, executor)
if err != nil {
return nil, util.WrapError(500, err)
}

return model, nil
}

func vchToModel(vch *vm.VirtualMachine, d *data.Data, executor *management.Dispatcher) (*models.VCH, error) {
func vchToModel(op trace.Operation, vch *vm.VirtualMachine, d *data.Data, executor *management.Dispatcher) (*models.VCH, error) {
vchConfig, err := executor.GetNoSecretVCHConfig(vch)
if err != nil {
return nil, fmt.Errorf("Unable to retrieve VCH information: %s", err)
Expand Down Expand Up @@ -236,10 +236,10 @@ func vchToModel(vch *vm.VirtualMachine, d *data.Data, executor *management.Dispa
model.Runtime = &models.VCHRuntime{}

installerVer := version.GetBuild()
upgradeStatus := upgradeStatusMessage(context.Background(), vch, installerVer, vchConfig.Version)
upgradeStatus := upgradeStatusMessage(op, vch, installerVer, vchConfig.Version)
model.Runtime.UpgradeStatus = upgradeStatus

powerState, err := vch.PowerState(context.Background())
powerState, err := vch.PowerState(op)
if err != nil {
powerState = "error"
}
Expand Down
18 changes: 10 additions & 8 deletions lib/apiservers/service/restapi/handlers/vch_list_get.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
package handlers

import (
"context"
"fmt"
"net/url"
"path"
Expand All @@ -28,6 +27,7 @@ import (
"github.com/vmware/vic/lib/apiservers/service/restapi/operations"
"github.com/vmware/vic/lib/install/data"
"github.com/vmware/vic/lib/install/management"
"github.com/vmware/vic/pkg/trace"
"github.com/vmware/vic/pkg/version"
"github.com/vmware/vic/pkg/vsphere/vm"
)
Expand All @@ -52,7 +52,8 @@ func (h *VCHListGet) Handle(params operations.GetTargetTargetVchParams, principa
return operations.NewGetTargetTargetVchDefault(util.StatusCode(err)).WithPayload(&models.Error{Message: err.Error()})
}

vchs, err := listVCHs(params.HTTPRequest.Context(), d)
op := trace.NewOperation(params.HTTPRequest.Context(), "vch_list get handler")
vchs, err := listVCHs(op, d)
if err != nil {
return operations.NewGetTargetTargetVchDefault(util.StatusCode(err)).WithPayload(&models.Error{Message: err.Error()})
}
Expand All @@ -72,16 +73,17 @@ func (h *VCHDatacenterListGet) Handle(params operations.GetTargetTargetDatacente
return operations.NewGetTargetTargetVchDefault(util.StatusCode(err)).WithPayload(&models.Error{Message: err.Error()})
}

vchs, err := listVCHs(params.HTTPRequest.Context(), d)
op := trace.NewOperation(params.HTTPRequest.Context(), "vch_list get handler")
vchs, err := listVCHs(op, d)
if err != nil {
return operations.NewGetTargetTargetVchDefault(util.StatusCode(err)).WithPayload(&models.Error{Message: err.Error()})
}

return operations.NewGetTargetTargetVchOK().WithPayload(operations.GetTargetTargetVchOKBody{Vchs: vchs})
}

func listVCHs(ctx context.Context, d *data.Data) ([]*models.VCHListItem, error) {
validator, err := validateTarget(ctx, d)
func listVCHs(op trace.Operation, d *data.Data) ([]*models.VCHListItem, error) {
validator, err := validateTarget(op, d)
if err != nil {
return nil, util.WrapError(400, err)
}
Expand All @@ -92,10 +94,10 @@ func listVCHs(ctx context.Context, d *data.Data) ([]*models.VCHListItem, error)
return nil, util.NewError(500, fmt.Sprintf("Failed to search VCHs in %s: %s", validator.ResourcePoolPath, err))
}

return vchsToModels(ctx, vchs, executor), nil
return vchsToModels(op, vchs, executor), nil
}

func vchsToModels(ctx context.Context, vchs []*vm.VirtualMachine, executor *management.Dispatcher) []*models.VCHListItem {
func vchsToModels(op trace.Operation, vchs []*vm.VirtualMachine, executor *management.Dispatcher) []*models.VCHListItem {
installerVer := version.GetBuild()
payload := make([]*models.VCHListItem, 0)
for _, vch := range vchs {
Expand Down Expand Up @@ -124,7 +126,7 @@ func vchsToModels(ctx context.Context, vchs []*vm.VirtualMachine, executor *mana

if version != nil {
model.Version = version.ShortVersion()
model.UpgradeStatus = upgradeStatusMessage(ctx, vch, installerVer, version)
model.UpgradeStatus = upgradeStatusMessage(op, vch, installerVer, version)
}

payload = append(payload, model)
Expand Down
Loading

0 comments on commit b4bacb0

Please sign in to comment.