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

Fix issue when setting instance group named ports #3

Merged
merged 1 commit into from
Oct 7, 2017
Merged

Fix issue when setting instance group named ports #3

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
9 changes: 5 additions & 4 deletions instances/instances.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ func (i *Instances) Init(zl zoneLister) {
// all of which have the exact same named ports.
func (i *Instances) AddInstanceGroup(name string, ports []int64) ([]*compute.InstanceGroup, []*compute.NamedPort, error) {
igs := []*compute.InstanceGroup{}
namedPorts := []*compute.NamedPort{}

var namedPorts []*compute.NamedPort
for _, port := range ports {
namedPorts = append(namedPorts, utils.GetNamedPort(port))
}
Expand Down Expand Up @@ -109,14 +110,14 @@ func (i *Instances) AddInstanceGroup(name string, ports []int64) ([]*compute.Ins
var newPorts []*compute.NamedPort
for _, np := range namedPorts {
if existingPorts[np.Port] {
glog.V(3).Infof("Instance group %v already has named port %+v", ig.Name, np)
glog.V(5).Infof("Instance group %v already has named port %+v", ig.Name, np)
continue
}
newPorts = append(newPorts, np)
}
if len(newPorts) > 0 {
glog.V(5).Infof("Instance group %v/%v does not have ports %+v, adding them now.", zone, name, namedPorts)
if err := i.cloud.SetNamedPortsOfInstanceGroup(ig.Name, zone, append(ig.NamedPorts, namedPorts...)); err != nil {
glog.V(3).Infof("Instance group %v/%v does not have ports %+v, adding them now.", zone, name, newPorts)
if err := i.cloud.SetNamedPortsOfInstanceGroup(ig.Name, zone, append(ig.NamedPorts, newPorts...)); err != nil {
return nil, nil, err
}
}
Expand Down
17 changes: 11 additions & 6 deletions instances/instances_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,30 +86,35 @@ func TestSetNamedPorts(t *testing.T) {
pool := newNodePool(f, defaultZone)

testCases := []struct {
newPorts []int64
activePorts []int64
expectedPorts []int64
}{
{
// Verify adding a port works as expected.
// Verify setting a port works as expected.
[]int64{80},
[]int64{80},
},
{
// Verify adding multiple ports at once works as expected.
// Utilizing multiple new ports
[]int64{81, 82},
[]int64{80, 81, 82},
},
{
// Adding existing ports should have no impact.
// Utilizing existing ports
[]int64{80, 82},
[]int64{80, 81, 82},
},
{
// Utilizing a new port and an old port
[]int64{80, 83},
[]int64{80, 81, 82, 83},
},
// TODO: Add tests to remove named ports when we support that.
}
for _, test := range testCases {
igs, _, err := pool.AddInstanceGroup("ig", test.newPorts)
igs, _, err := pool.AddInstanceGroup("ig", test.activePorts)
if err != nil {
t.Fatalf("unexpected error in adding ports %v to instance group: %s", test.newPorts, err)
t.Fatalf("unexpected error in setting ports %v to instance group: %s", test.activePorts, err)
}
if len(igs) != 1 {
t.Fatalf("expected a single instance group, got: %v", igs)
Expand Down