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

A huge migration to a newer swagger. #3481

Merged
merged 12 commits into from
Dec 13, 2016
Merged

A huge migration to a newer swagger. #3481

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
922b081
Huge migration to a newer swagger.
vburenin Dec 9, 2016
a233d31
Update Makefile according to swagger changes.
vburenin Dec 10, 2016
7ede85f
Swagger migration pure code related changes.
vburenin Dec 12, 2016
7b49636
All new swagger vendor code.
vburenin Dec 12, 2016
d360ed4
Merge branch 'swagger-update' of github.com:vburenin/vic into swagger…
vburenin Dec 12, 2016
7988621
Revert "Swagger migration pure code related changes."
vburenin Dec 12, 2016
737c885
Revert "All new swagger vendor code."
vburenin Dec 12, 2016
3445742
Updated Makefile.
vburenin Dec 12, 2016
402830e
Provide a user hook for api servers configuration in swagger.
vburenin Dec 13, 2016
8110091
Use api server config hooks to set custom shutfown workflow,.
vburenin Dec 13, 2016
b352166
Merge branch 'master' into swagger-update
vburenin Dec 13, 2016
142f93b
Merge branch 'master' into swagger-update
vburenin Dec 13, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
27 changes: 19 additions & 8 deletions cmd/port-layer-server/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,13 @@ import (
"os/signal"
"syscall"

spec "github.com/go-swagger/go-swagger/spec"
flags "github.com/jessevdk/go-flags"
"github.com/go-openapi/loads"
"github.com/jessevdk/go-flags"

"github.com/vmware/vic/lib/apiservers/portlayer/restapi"
"github.com/vmware/vic/lib/apiservers/portlayer/restapi/operations"
"github.com/vmware/vic/lib/pprof"

"github.com/vmware/vic/lib/dns"
"github.com/vmware/vic/lib/pprof"
)

var (
Expand All @@ -39,7 +38,8 @@ func init() {
}

func main() {
swaggerSpec, err := spec.New(restapi.SwaggerJSON, "")

swaggerSpec, err := loads.Analyzed(restapi.SwaggerJSON, "")
if err != nil {
log.Fatalln(err)
}
Expand All @@ -53,19 +53,30 @@ func main() {
parser.LongDescription = `Port Layer API`

server.ConfigureFlags()

for _, optsGroup := range api.CommandLineOptionsGroups {
parser.AddGroup(optsGroup.ShortDescription, optsGroup.LongDescription, optsGroup.Options)
_, err := parser.AddGroup(optsGroup.ShortDescription, optsGroup.LongDescription, optsGroup.Options)
if err != nil {
log.Fatalln(err)
}
}

if _, err := parser.Parse(); err != nil {
os.Exit(1)
code := 1
if fe, ok := err.(*flags.Error); ok {
if fe.Type == flags.ErrHelp {
code = 0
}
}
os.Exit(code)
}

server.ConfigureAPI()

// BEGIN
// Set the Interface name to instruct listeners to bind on this interface
options.Interface = "bridge"

// Start the DNS Server
dnsserver := dns.NewServer(options)
if dnsserver != nil {
Expand All @@ -81,7 +92,7 @@ func main() {

dnsserver.Stop()

server.Stop()
server.Shutdown()
}()

go func() {
Expand Down
41 changes: 21 additions & 20 deletions lib/apiservers/engine/backends/backends.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,12 @@ import (

log "github.com/Sirupsen/logrus"
"github.com/docker/docker/registry"
httptransport "github.com/go-swagger/go-swagger/httpkit/client"
"github.com/go-swagger/go-swagger/swag"
rc "github.com/go-openapi/runtime/client"
"github.com/go-openapi/swag"

"github.com/vmware/vic/lib/apiservers/engine/backends/cache"
"github.com/vmware/vic/lib/apiservers/engine/backends/container"
"github.com/vmware/vic/lib/apiservers/portlayer/client"
apiclient "github.com/vmware/vic/lib/apiservers/portlayer/client"
Copy link
Contributor

Choose a reason for hiding this comment

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

apiclient isn't really necessary to migrate from 1 swagger version to another. We can debate the naming of this later. Lets keep this change restricted only only the required changes to include the new swagger version. The unnecessary churn makes reviewing harder.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I used tools to deal with imports, all automatic changes are not easy to manage manually.

"github.com/vmware/vic/lib/apiservers/portlayer/client/containers"
"github.com/vmware/vic/lib/apiservers/portlayer/client/misc"
"github.com/vmware/vic/lib/apiservers/portlayer/client/scopes"
Expand All @@ -52,7 +52,7 @@ const (
)

var (
portLayerClient *client.PortLayer
portLayerClient *apiclient.PortLayer
portLayerServerAddr string
portLayerName string
productName string
Expand Down Expand Up @@ -88,8 +88,9 @@ func Init(portLayerAddr, product string, config *config.VirtualContainerHostConf
portLayerName = product + " Backend Engine"
}

t := httptransport.New(portLayerAddr, "/", []string{"http"})
portLayerClient = client.New(t, nil)
t := rc.New(portLayerAddr, "/", []string{"http"})

portLayerClient = apiclient.New(t, nil)
portLayerServerAddr = portLayerAddr

// block indefinitely while waiting on the portlayer to respond to pings
Expand Down Expand Up @@ -123,52 +124,52 @@ func hydrateCaches() error {

wg := sync.WaitGroup{}
wg.Add(waiters)
errors := make(chan error, waiters)
errChan := make(chan error, waiters)
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment as above. This isn't strictly necessary to migrate to the new swagger version. Please remove it from your change and we can review these cosmetic changes in a different PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the cosmetic change and the only one that you can say "big".


go func() {
defer wg.Done()
if err := imagec.InitializeLayerCache(portLayerClient); err != nil {
errors <- fmt.Errorf("Failed to initialize layer cache: %s", err)
errChan <- fmt.Errorf("Failed to initialize layer cache: %s", err)
return
}
log.Info("Layer cache initialized successfully")
errors <- nil
errChan <- nil
}()

go func() {
defer wg.Done()
if err := cache.InitializeImageCache(portLayerClient); err != nil {
errors <- fmt.Errorf("Failed to initialize image cache: %s", err)
errChan <- fmt.Errorf("Failed to initialize image cache: %s", err)
return
}
log.Info("Image cache initialized successfully")

// container cache relies on image cache so we share a goroutine to update
// them serially
if err := syncContainerCache(); err != nil {
errors <- fmt.Errorf("Failed to update container cache: %s", err)
errChan <- fmt.Errorf("Failed to update container cache: %s", err)
return
}
log.Info("Container cache updated successfully")
errors <- nil
errChan <- nil
}()

go func() {
log.Info("Refreshing repository cache")
defer wg.Done()
if err := cache.NewRepositoryCache(portLayerClient); err != nil {
errors <- fmt.Errorf("Failed to create repository cache: %s", err.Error())
errChan <- fmt.Errorf("Failed to create repository cache: %s", err.Error())
return
}
errors <- nil
errChan <- nil
log.Info("Repository cache updated successfully")
}()

wg.Wait()
close(errors)
close(errChan)

var errs []string
for err := range errors {
for err := range errChan {
if err != nil {
// accumulate all errors into one
errs = append(errs, err.Error())
Expand All @@ -182,7 +183,7 @@ func hydrateCaches() error {
return e
}

func PortLayerClient() *client.PortLayer {
func PortLayerClient() *apiclient.PortLayer {
return portLayerClient
}

Expand Down Expand Up @@ -285,12 +286,12 @@ func syncContainerCache() error {
}

func setPortMapping(info *models.ContainerInfo, backend *Container, container *container.VicContainer) error {
if info.ContainerConfig.State == nil {
if info.ContainerConfig.State == "" {
log.Infof("container state is nil")
return nil
}
if *info.ContainerConfig.State != "Running" || len(container.HostConfig.PortBindings) == 0 {
log.Infof("Container info state: %s", *info.ContainerConfig.State)
if info.ContainerConfig.State != "Running" || len(container.HostConfig.PortBindings) == 0 {
log.Infof("Container info state: %s", info.ContainerConfig.State)
log.Infof("container portbinding: %+v", container.HostConfig.PortBindings)
return nil
}
Expand Down
60 changes: 36 additions & 24 deletions lib/apiservers/engine/backends/container.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package backends

import (
"context"
"fmt"
"io"
"math/rand"
Expand All @@ -25,8 +26,6 @@ import (
"sync"
"time"

"context"

log "github.com/Sirupsen/logrus"
"github.com/cenkalti/backoff"
"github.com/docker/docker/api/types/backend"
Expand Down Expand Up @@ -985,22 +984,25 @@ func (c *Container) ContainerInspect(name string, size bool, version version.Ver
}
var started time.Time
var stopped time.Time
if results.Payload.ProcessConfig.StartTime != nil && *results.Payload.ProcessConfig.StartTime > 0 {
started = time.Unix(*results.Payload.ProcessConfig.StartTime, 0)

if results.Payload.ProcessConfig.StartTime > 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this purely formatting? Unnecessary churn to migrate to the new swagger. Encapsulate these cosmetic changes in their own PR and we can review them later.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is not a cosmetic change. it a result of a variable becoming a value not a pointer during migration.

Copy link
Contributor

Choose a reason for hiding this comment

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

@fdawg4l There are a few cosmetic changes, but I would gloss over them and focus on the pointer issue. Vast majority of the changes I've seen so far are due to Swagger and not cosmetic.

started = time.Unix(results.Payload.ProcessConfig.StartTime, 0)
}
if results.Payload.ProcessConfig.StopTime != nil && *results.Payload.ProcessConfig.StopTime > 0 {
stopped = time.Unix(*results.Payload.ProcessConfig.StopTime, 0)
if results.Payload.ProcessConfig.StopTime > 0 {
stopped = time.Unix(results.Payload.ProcessConfig.StopTime, 0)
}

// call to the dockerStatus function to retrieve the docker friendly exitCode
exitCode, status := dockerStatus(int(*results.Payload.ProcessConfig.ExitCode),
*results.Payload.ProcessConfig.Status,
*results.Payload.ContainerConfig.State,
exitCode, status := dockerStatus(
int(results.Payload.ProcessConfig.ExitCode),
results.Payload.ProcessConfig.Status,
results.Payload.ContainerConfig.State,
started, stopped)

// set the payload values
exit := int32(exitCode)
results.Payload.ProcessConfig.ExitCode = &exit
results.Payload.ProcessConfig.Status = &status
results.Payload.ProcessConfig.ExitCode = exit
results.Payload.ProcessConfig.Status = status

inspectJSON, err := ContainerInfoToDockerContainerInspect(vc, results.Payload, PortLayerName())
if err != nil {
Expand Down Expand Up @@ -1098,14 +1100,22 @@ func (c *Container) Containers(config *types.ContainerListOptions) ([]*types.Con
}
var started time.Time
var stopped time.Time
if t.ProcessConfig.StartTime != nil && *t.ProcessConfig.StartTime > 0 {
started = time.Unix(*t.ProcessConfig.StartTime, 0)

if t.ProcessConfig.StartTime > 0 {
started = time.Unix(t.ProcessConfig.StartTime, 0)
}
if t.ProcessConfig.StopTime != nil && *t.ProcessConfig.StopTime > 0 {
stopped = time.Unix(*t.ProcessConfig.StopTime, 0)

if t.ProcessConfig.StopTime > 0 {
stopped = time.Unix(t.ProcessConfig.StopTime, 0)
}

// get the docker friendly status
_, status := dockerStatus(int(*t.ProcessConfig.ExitCode), *t.ProcessConfig.Status, *t.ContainerConfig.State, started, stopped)
_, status := dockerStatus(
int(t.ProcessConfig.ExitCode),
t.ProcessConfig.Status,
t.ContainerConfig.State,
started,
stopped)

ips, err := publicIPv4Addrs()
var ports []types.Port
Expand All @@ -1123,7 +1133,7 @@ func (c *Container) Containers(config *types.ContainerListOptions) ([]*types.Con
imageID, err := cache.RepositoryCache().Get(ref)
if err != nil && err == cache.ErrDoesNotExist {
// the tag has been removed, so we need to show the truncated imageID
imageID = cache.RepositoryCache().GetImageID(*t.ContainerConfig.LayerID)
imageID = cache.RepositoryCache().GetImageID(t.ContainerConfig.LayerID)
if imageID != "" {
id := uid.Parse(imageID)
repo = id.Truncate().String()
Expand All @@ -1132,13 +1142,13 @@ func (c *Container) Containers(config *types.ContainerListOptions) ([]*types.Con
}

c := &types.Container{
ID: *t.ContainerConfig.ContainerID,
ID: t.ContainerConfig.ContainerID,
Image: repo,
Created: *t.ContainerConfig.CreateTime,
Created: t.ContainerConfig.CreateTime,
Status: status,
Names: names,
Command: cmd,
SizeRw: *t.ContainerConfig.StorageSize,
SizeRw: t.ContainerConfig.StorageSize,
Ports: ports,
}
containers = append(containers, c)
Expand Down Expand Up @@ -1521,17 +1531,19 @@ func portInformation(t *models.ContainerInfo, ips []netlink.Addr) []types.Port {
// (works with both IPv4 and IPv6 addresses)
var ports []types.Port

container := cache.ContainerCache().GetContainer(*t.ContainerConfig.ContainerID)
if container == nil {
log.Errorf("Could not find container with ID %s", *t.ContainerConfig.ContainerID)
cid := t.ContainerConfig.ContainerID
c := cache.ContainerCache().GetContainer(cid)
Copy link
Contributor

Choose a reason for hiding this comment

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

+1. I really hate go programmer's habit of overloading one line with a bazillion operations. They need to remember the compiler doesn't care, and readability is more important.


if c == nil {
log.Errorf("Could not find container with ID %s", cid)
return ports
}

for _, ip := range ips {
ports = append(ports, types.Port{IP: ip.IP.String()})
}

portBindings := container.HostConfig.PortBindings
portBindings := c.HostConfig.PortBindings
var resultPorts []types.Port
var err error

Expand Down
Loading