Repository: helm/helm
Pull request #8505 (2020-07-23 04:42:52)

Avoid hardcoded container port in default notes
<!--  Thanks for sending a pull request!  Here are some tips for you:
1. Make sure to read the Contributing Guide before submitting your PR: https://github.com/helm/helm/blob/master/CONTRIBUTING.md
2. If this PR closes another issue, add 'closes #<issue number>' somewhere in the PR summary. GitHub will automatically close that issue when this PR gets merged. Alternatively, adding 'refs #<issue number>' will not close the issue, but help provide the reviewer more context.-->

**What this PR does / why we need it**:

Currently in the default notes generated by `helm create`, the container port used in `port-forward` is hard-coded to `80`. This is confusing to the user when the actual container port is set to something else. This PR fixes this by adding a line to query the actual container port in the script generated after `helm install`.

Ref: https://kubernetes.io/docs/tasks/access-application-cluster/port-forward-access-application-cluster/

**Special notes for your reviewer**:

Nil

**If applicable**:
- [ ] this PR contains documentation
- [ ] this PR contains unit tests
- [x] this PR has been tested for backwards compatibility



Patch:
diff --git a/cmd/helm/dependency_build.go b/cmd/helm/dependency_build.go
index 4e87684ce..478b49479 100644
--- a/cmd/helm/dependency_build.go
+++ b/cmd/helm/dependency_build.go
@@ -16,7 +16,6 @@ limitations under the License.
 package main
 
 import (
-	"fmt"
 	"io"
 	"os"
 	"path/filepath"
@@ -66,11 +65,7 @@ func newDependencyBuildCmd(out io.Writer) *cobra.Command {
 			if client.Verify {
 				man.Verify = downloader.VerifyIfPossible
 			}
-			err := man.Build()
-			if e, ok := err.(downloader.ErrRepoNotFound); ok {
-				return fmt.Errorf("%s. Please add the missing repos via 'helm repo add'", e.Error())
-			}
-			return err
+			return man.Build()
 		},
 	}
 
diff --git a/pkg/chartutil/create.go b/pkg/chartutil/create.go
index c036ce37d..6e382b961 100644
--- a/pkg/chartutil/create.go
+++ b/pkg/chartutil/create.go
@@ -381,8 +381,9 @@ const defaultNotes = `1. Get the application URL by running these commands:
   echo http://$SERVICE_IP:{{ .Values.service.port }}
 {{- else if contains "ClusterIP" .Values.service.type }}
   export POD_NAME=$(kubectl get pods --namespace {{ .Release.Namespace }} -l "app.kubernetes.io/name={{ include "<CHARTNAME>.name" . }},app.kubernetes.io/instance={{ .Release.Name }}" -o jsonpath="{.items[0].metadata.name}")
+  export CONTAINER_PORT=$(kubectl get pod --namespace {{ .Release.Namespace }} $POD_NAME -o jsonpath="{.spec.containers[0].ports[0].containerPort}")
   echo "Visit http://127.0.0.1:8080 to use your application"
-  kubectl --namespace {{ .Release.Namespace }} port-forward $POD_NAME 8080:80
+  kubectl --namespace {{ .Release.Namespace }} port-forward $POD_NAME 8080:$CONTAINER_PORT
 {{- end }}
 `
 
diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go
index 2d6918739..00198de0c 100644
--- a/pkg/downloader/manager.go
+++ b/pkg/downloader/manager.go
@@ -42,17 +42,6 @@ import (
 	"helm.sh/helm/v3/pkg/repo"
 )
 
-// ErrRepoNotFound indicates that chart repositories can't be found in local repo cache.
-// The value of Repos is missing repos.
-type ErrRepoNotFound struct {
-	Repos []string
-}
-
-// Error implements the error interface.
-func (e ErrRepoNotFound) Error() string {
-	return fmt.Sprintf("no repository definition for %s", strings.Join(e.Repos, ", "))
-}
-
 // Manager handles the lifecycle of fetching, resolving, and storing dependencies.
 type Manager struct {
 	// Out is used to print warnings and notifications.
@@ -422,7 +411,7 @@ Loop:
 		missing = append(missing, dd.Repository)
 	}
 	if len(missing) > 0 {
-		return ErrRepoNotFound{missing}
+		return errors.Errorf("no repository definition for %s. Please add the missing repos via 'helm repo add'", strings.Join(missing, ", "))
 	}
 	return nil
 }


Test patch:
diff --git a/pkg/downloader/manager_test.go b/pkg/downloader/manager_test.go
index dd3cbbd7b..ea235c13f 100644
--- a/pkg/downloader/manager_test.go
+++ b/pkg/downloader/manager_test.go
@@ -360,32 +360,3 @@ func TestBuild_WithRepositoryAlias(t *testing.T) {
 		Repository: "@test",
 	})
 }
-
-func TestErrRepoNotFound_Error(t *testing.T) {
-	type fields struct {
-		Repos []string
-	}
-	tests := []struct {
-		name   string
-		fields fields
-		want   string
-	}{
-		{
-			name: "OK",
-			fields: fields{
-				Repos: []string{"https://charts1.example.com", "https://charts2.example.com"},
-			},
-			want: "no repository definition for https://charts1.example.com, https://charts2.example.com",
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			e := ErrRepoNotFound{
-				Repos: tt.fields.Repos,
-			}
-			if got := e.Error(); got != tt.want {
-				t.Errorf("Error() = %v, want %v", got, tt.want)
-			}
-		})
-	}
-}