diff --git a/pkg/skaffold/kubernetes/portforward/entry_manager_test.go b/pkg/skaffold/kubernetes/portforward/entry_manager_test.go index b614ecb61fd..1f2ba7e0b7e 100644 --- a/pkg/skaffold/kubernetes/portforward/entry_manager_test.go +++ b/pkg/skaffold/kubernetes/portforward/entry_manager_test.go @@ -48,22 +48,17 @@ func TestNewEntryManager(t *testing.T) { } func TestStop(t *testing.T) { - pfe1 := &portForwardEntry{ - resource: latest.PortForwardResource{ - Type: constants.Pod, - Name: "resource", - Namespace: "default", - }, - localPort: 9000, - } - pfe2 := &portForwardEntry{ - resource: latest.PortForwardResource{ - Type: constants.Pod, - Name: "resource2", - Namespace: "default", - }, - localPort: 9001, - } + pfe1 := newPortForwardEntry(0, latest.PortForwardResource{ + Type: constants.Pod, + Name: "resource", + Namespace: "default", + }, "", "", "", 9000, false) + + pfe2 := newPortForwardEntry(0, latest.PortForwardResource{ + Type: constants.Pod, + Name: "resource2", + Namespace: "default", + }, "", "", "", 9001, false) em := NewEntryManager(ioutil.Discard, nil) diff --git a/pkg/skaffold/kubernetes/portforward/pod_forwarder.go b/pkg/skaffold/kubernetes/portforward/pod_forwarder.go index f276ba8f289..159a2bf9f57 100644 --- a/pkg/skaffold/kubernetes/portforward/pod_forwarder.go +++ b/pkg/skaffold/kubernetes/portforward/pod_forwarder.go @@ -19,7 +19,6 @@ package portforward import ( "context" "strconv" - "sync" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/color" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants" @@ -138,15 +137,7 @@ func (p *WatchingPodForwarder) podForwardingEntry(resourceVersion, containerName if err != nil { return nil, errors.Wrap(err, "converting resource version to integer") } - entry := &portForwardEntry{ - resource: resource, - resourceVersion: rv, - podName: resource.Name, - containerName: containerName, - portName: portName, - automaticPodForwarding: true, - terminationLock: &sync.Mutex{}, - } + entry := newPortForwardEntry(rv, resource, resource.Name, containerName, portName, 0, true) // If we have, return the current entry oldEntry, ok := p.forwardedResources.Load(entry.key()) diff --git a/pkg/skaffold/kubernetes/portforward/port_forward_entry.go b/pkg/skaffold/kubernetes/portforward/port_forward_entry.go index 420aa068a44..bf2439ba6c4 100644 --- a/pkg/skaffold/kubernetes/portforward/port_forward_entry.go +++ b/pkg/skaffold/kubernetes/portforward/port_forward_entry.go @@ -37,6 +37,20 @@ type portForwardEntry struct { cancel context.CancelFunc } +// newPortForwardEntry returns a port forward entry. +func newPortForwardEntry(resourceVersion int, resource latest.PortForwardResource, podName, containerName, portName string, localPort int, automaticPodForwarding bool) *portForwardEntry { + return &portForwardEntry{ + resourceVersion: resourceVersion, + resource: resource, + podName: podName, + containerName: containerName, + portName: portName, + localPort: localPort, + automaticPodForwarding: automaticPodForwarding, + terminationLock: &sync.Mutex{}, + } +} + // key is an identifier for the lock on a port during the skaffold dev cycle. // if automaticPodForwarding is set, we return a key that doesn't include podName, since we want the key // to be the same whenever pods restart diff --git a/pkg/skaffold/kubernetes/portforward/port_forward_entry_test.go b/pkg/skaffold/kubernetes/portforward/port_forward_entry_test.go index 200b3f5190b..0e730bff31a 100644 --- a/pkg/skaffold/kubernetes/portforward/port_forward_entry_test.go +++ b/pkg/skaffold/kubernetes/portforward/port_forward_entry_test.go @@ -32,25 +32,21 @@ func TestPortForwardEntryKey(t *testing.T) { }{ { description: "entry for pod", - pfe: &portForwardEntry{ - resource: latest.PortForwardResource{ - Type: "pod", - Name: "podName", - Namespace: "default", - Port: 8080, - }, - }, + pfe: newPortForwardEntry(0, latest.PortForwardResource{ + Type: "pod", + Name: "podName", + Namespace: "default", + Port: 8080, + }, "", "", "", 0, false), expected: "pod-podName-default-8080", }, { description: "entry for deploy", - pfe: &portForwardEntry{ - resource: latest.PortForwardResource{ - Type: "deployment", - Name: "depName", - Namespace: "namespace", - Port: 9000, - }, - }, + pfe: newPortForwardEntry(0, latest.PortForwardResource{ + Type: "deployment", + Name: "depName", + Namespace: "namespace", + Port: 9000, + }, "", "", "", 0, false), expected: "deployment-depName-namespace-9000", }, } @@ -78,17 +74,12 @@ func TestAutomaticPodForwardingKey(t *testing.T) { }{ { description: "entry for automatically port forwarded pod", - pfe: &portForwardEntry{ - containerName: "containerName", - portName: "portName", - resource: latest.PortForwardResource{ - Type: "pod", - Name: "podName", - Namespace: "default", - Port: 8080, - }, - automaticPodForwarding: true, - }, + pfe: newPortForwardEntry(0, latest.PortForwardResource{ + Type: "pod", + Name: "podName", + Namespace: "default", + Port: 8080, + }, "", "containerName", "portName", 0, true), expected: "containerName-default-portName-8080", }, } diff --git a/pkg/skaffold/kubernetes/portforward/port_forward_integration.go b/pkg/skaffold/kubernetes/portforward/port_forward_integration.go index 66c3d1c9032..fb6d885b183 100644 --- a/pkg/skaffold/kubernetes/portforward/port_forward_integration.go +++ b/pkg/skaffold/kubernetes/portforward/port_forward_integration.go @@ -19,7 +19,6 @@ package portforward import ( "context" "os" - "sync" "testing" "time" @@ -38,18 +37,12 @@ func WhiteBoxPortForwardCycle(t *testing.T, kubectlCLI *kubectl.CLI, namespace s portForwardEvent = func(entry *portForwardEntry) {} ctx := context.Background() localPort := retrieveAvailablePort(9000, em.forwardedPorts) - pfe := &portForwardEntry{ - resource: latest.PortForwardResource{ - Type: "deployment", - Name: "leeroy-web", - Namespace: namespace, - Port: 8080, - }, - containerName: "dummy container", - localPort: localPort, - terminationLock: &sync.Mutex{}, - } - + pfe := newPortForwardEntry(0, latest.PortForwardResource{ + Type: "deployment", + Name: "leeroy-web", + Namespace: namespace, + Port: 8080, + }, "", "dummy container", "", localPort, false) defer em.Stop() em.forwardPortForwardEntry(ctx, pfe) em.Stop() diff --git a/pkg/skaffold/kubernetes/portforward/resource_forwarder.go b/pkg/skaffold/kubernetes/portforward/resource_forwarder.go index f7754ed2db7..6bc26f83179 100644 --- a/pkg/skaffold/kubernetes/portforward/resource_forwarder.go +++ b/pkg/skaffold/kubernetes/portforward/resource_forwarder.go @@ -18,7 +18,6 @@ package portforward import ( "context" - "sync" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes" @@ -81,10 +80,8 @@ func (p *ResourceForwarder) portForwardResource(ctx context.Context, resource la func (p *ResourceForwarder) getCurrentEntry(resource latest.PortForwardResource) *portForwardEntry { // determine if we have seen this before - entry := &portForwardEntry{ - resource: resource, - terminationLock: &sync.Mutex{}, - } + entry := newPortForwardEntry(0, resource, "", "", "", 0, false) + // If we have, return the current entry oldEntry, ok := p.forwardedResources.Load(entry.key()) diff --git a/pkg/skaffold/kubernetes/portforward/resource_forwarder_test.go b/pkg/skaffold/kubernetes/portforward/resource_forwarder_test.go index 283635da82c..bc7f1e6b72e 100644 --- a/pkg/skaffold/kubernetes/portforward/resource_forwarder_test.go +++ b/pkg/skaffold/kubernetes/portforward/resource_forwarder_test.go @@ -154,10 +154,7 @@ func TestGetCurrentEntryFunc(t *testing.T) { Port: 8080, }, availablePorts: []int{8080}, - expected: &portForwardEntry{ - localPort: 8080, - terminationLock: &sync.Mutex{}, - }, + expected: newPortForwardEntry(0, latest.PortForwardResource{}, "", "", "", 8080, false), }, { description: "port forward existing deployment", resource: latest.PortForwardResource{ @@ -177,10 +174,7 @@ func TestGetCurrentEntryFunc(t *testing.T) { localPort: 9000, }, }, - expected: &portForwardEntry{ - localPort: 9000, - terminationLock: &sync.Mutex{}, - }, + expected: newPortForwardEntry(0, latest.PortForwardResource{}, "", "", "", 9000, false), }, }