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

Remove unused named ports from instance group's #430

Merged
merged 1 commit into from
Aug 21, 2018
+16 −21
Merged

Remove unused named ports from instance group's #430

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
7 changes: 3 additions & 4 deletions pkg/instances/instances.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,12 +117,11 @@ func (i *Instances) ensureInstanceGroupAndPorts(name, zone string, ports []int64
existingPorts[np.Port] = true
}

// Determine which ports need to be added
// Determine which ports need to be added. Note that this adds existing ports as well.
var newPorts []int64
for _, p := range ports {
if existingPorts[p] {
glog.V(5).Infof("Instance group %v/%v already has named port %v", zone, ig.Name, p)
continue
}
newPorts = append(newPorts, p)
}
Expand All @@ -134,8 +133,8 @@ func (i *Instances) ensureInstanceGroupAndPorts(name, zone string, ports []int64
}

if len(newNamedPorts) > 0 {
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, newNamedPorts...)); err != nil {
glog.V(3).Infof("Setting named ports %+v on instance group %v/%v", zone, name, newPorts)
if err := i.cloud.SetNamedPortsOfInstanceGroup(ig.Name, zone, newNamedPorts); err != nil {
return nil, err
}
}
Expand Down
30 changes: 13 additions & 17 deletions pkg/instances/instances_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,43 +86,39 @@ func TestSetNamedPorts(t *testing.T) {
f := NewFakeInstanceGroups(sets.NewString([]string{"ig"}...), defaultNamer)
pool := newNodePool(f, defaultZone)

// Note: each test case is dependent on the previous.
testCases := []struct {
activePorts []int64
desc string
expectedPorts []int64
}{
{
// Verify setting a port works as expected.
[]int64{80},
[]int64{80},
desc: "Set single port",
expectedPorts: []int64{80},
},
{
// Utilizing multiple new ports
[]int64{81, 82},
[]int64{80, 81, 82},
desc: "Two new ports + existing port",
expectedPorts: []int64{80, 81, 82},
},
{
// Utilizing existing ports
[]int64{80, 82},
[]int64{80, 81, 82},
desc: "Utilize all existing ports",
expectedPorts: []int64{80, 81, 82},
},
{
// Utilizing a new port and an old port
[]int64{80, 83},
[]int64{80, 81, 82, 83},
desc: "Two new ports + remove unused port",
expectedPorts: []int64{81, 82, 83, 84},
},
// TODO: Add tests to remove named ports when we support that.
}
for _, test := range testCases {
igs, err := pool.EnsureInstanceGroupsAndPorts("ig", test.activePorts)
igs, err := pool.EnsureInstanceGroupsAndPorts("ig", test.expectedPorts)
if err != nil {
t.Fatalf("unexpected error in setting ports %v to instance group: %s", test.activePorts, err)
t.Fatalf("unexpected error in setting ports %v to instance group: %s", test.expectedPorts, err)
}
if len(igs) != 1 {
t.Fatalf("expected a single instance group, got: %v", igs)
}
actualPorts := igs[0].NamedPorts
if len(actualPorts) != len(test.expectedPorts) {
t.Fatalf("unexpected named ports on instance group. expected: %v, got: %v", test.expectedPorts, actualPorts)
t.Fatalf("unexpected number of named ports on instance group. expected: %v, got: %v", len(test.expectedPorts), len(actualPorts))
}
for i, p := range igs[0].NamedPorts {
if p.Port != test.expectedPorts[i] {
Expand Down