From 213d6b77767656e303a3e10c6713a69376dfe38b Mon Sep 17 00:00:00 2001 From: Nick Sardo Date: Tue, 1 Aug 2017 17:56:14 -0700 Subject: [PATCH 1/2] Better handling of race when creating instance groups --- controllers/gce/instances/instances.go | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/controllers/gce/instances/instances.go b/controllers/gce/instances/instances.go index eea112d81..b38c1b656 100644 --- a/controllers/gce/instances/instances.go +++ b/controllers/gce/instances/instances.go @@ -73,19 +73,27 @@ func (i *Instances) AddInstanceGroup(name string, port int64) ([]*compute.Instan defer i.snapshotter.Add(name, struct{}{}) for _, zone := range zones { - ig, _ := i.Get(name, zone) - var err error + ig, err := i.Get(name, zone) + if err != nil && !utils.IsHTTPErrorCode(err, http.StatusNotFound) { + glog.Errorf("Failed to get instance group %v/%v, err: %v", zone, name, err) + return nil, nil, err + } + if ig == nil { glog.Infof("Creating instance group %v in zone %v", name, zone) if err = i.cloud.CreateInstanceGroup(&compute.InstanceGroup{Name: name}, zone); err != nil { - return nil, nil, err + if utils.IsHTTPErrorCode(err, http.StatusConflict) { + glog.Warningf("Failed to create instance group %v/%v due to conflict status, but continuing sync. err: %v", zone, name, err) + } else { + glog.Errorf("Failed to create instance group %v/%v, err: %v", zone, name, err) + return nil, nil, err + } } ig, err = i.cloud.GetInstanceGroup(name, zone) if err != nil { + glog.Errorf("Failed to get instance group %v/%v after ensuring existence, err: %v", zone, name, err) return nil, nil, err } - } else { - glog.V(3).Infof("Instance group %v already exists in zone %v, adding port %d to it", name, zone, port) } found := false From bec28ccf86f483d2c13ae162c2ef8b5a779fe9e6 Mon Sep 17 00:00:00 2001 From: Nick Sardo Date: Wed, 2 Aug 2017 13:16:44 -0700 Subject: [PATCH 2/2] Adding few more log messages and comment --- controllers/gce/instances/instances.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/controllers/gce/instances/instances.go b/controllers/gce/instances/instances.go index b38c1b656..14132c6fb 100644 --- a/controllers/gce/instances/instances.go +++ b/controllers/gce/instances/instances.go @@ -82,6 +82,8 @@ func (i *Instances) AddInstanceGroup(name string, port int64) ([]*compute.Instan if ig == nil { glog.Infof("Creating instance group %v in zone %v", name, zone) if err = i.cloud.CreateInstanceGroup(&compute.InstanceGroup{Name: name}, zone); err != nil { + // Error may come back with StatusConflict meaning the instance group was created by another controller + // possibly the Service Controller for internal load balancers. if utils.IsHTTPErrorCode(err, http.StatusConflict) { glog.Warningf("Failed to create instance group %v/%v due to conflict status, but continuing sync. err: %v", zone, name, err) } else { @@ -94,6 +96,8 @@ func (i *Instances) AddInstanceGroup(name string, port int64) ([]*compute.Instan glog.Errorf("Failed to get instance group %v/%v after ensuring existence, err: %v", zone, name, err) return nil, nil, err } + } else { + glog.V(3).Infof("Instance group %v already exists in zone %v", name, zone) } found := false @@ -105,6 +109,7 @@ func (i *Instances) AddInstanceGroup(name string, port int64) ([]*compute.Instan } } if !found { + glog.V(3).Infof("Instance group %v/%v does not have port %+v, adding it now.", zone, name, namedPort) if err := i.cloud.SetNamedPortsOfInstanceGroup(ig.Name, zone, append(ig.NamedPorts, namedPort)); err != nil { return nil, nil, err }