Merge pull request #900 from sargun/mov-env

Move env var code into its own package

internal/podutils/env.go

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package node
+package podutils
 
 import (
 	"context"
@@ -71,9 +71,8 @@ const (
 
 var masterServices = sets.NewString("kubernetes")
 
-// populateEnvironmentVariables populates the environment of each container (and init container) in the specified pod.
+// PopulateEnvironmentVariables populates the environment of each container (and init container) in the specified pod.
-// TODO Make this the single exported function of a "pkg/environment" package in the future.
+func PopulateEnvironmentVariables(ctx context.Context, pod *corev1.Pod, rm *manager.ResourceManager, recorder record.EventRecorder) error {
-func populateEnvironmentVariables(ctx context.Context, pod *corev1.Pod, rm *manager.ResourceManager, recorder record.EventRecorder) error {
 
 	// Populate each init container's environment.
 	for idx := range pod.Spec.InitContainers {

internal/podutils/env_internal_test.go

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package node
+package podutils
 
 import (
 	"context"
@@ -215,7 +215,7 @@ func TestPopulatePodWithInitContainersUsingEnv(t *testing.T) {
 	}
 
 	// Populate the pod's environment.
-	err := populateEnvironmentVariables(context.Background(), pod, rm, er)
+	err := PopulateEnvironmentVariables(context.Background(), pod, rm, er)
 	assert.Check(t, err)
 
 	// Make sure that all the containers' environments contain all the expected keys and values.
@@ -375,7 +375,7 @@ func TestPopulatePodWithInitContainersUsingEnvWithFieldRef(t *testing.T) {
 	}
 
 	// Populate the pod's environment.
-	err := populateEnvironmentVariables(context.Background(), pod, rm, er)
+	err := PopulateEnvironmentVariables(context.Background(), pod, rm, er)
 	assert.NilError(t, err)
 
 	// Make sure that all the containers' environments contain all the expected keys and values.
@@ -491,7 +491,7 @@ func TestPopulatePodWithInitContainersUsingEnvFrom(t *testing.T) {
 	}
 
 	// Populate the pod's environment.
-	err := populateEnvironmentVariables(context.Background(), pod, rm, er)
+	err := PopulateEnvironmentVariables(context.Background(), pod, rm, er)
 	assert.Check(t, err)
 
 	// Make sure that all the containers' environments contain all the expected keys and values.
@@ -632,7 +632,7 @@ func TestEnvFromConfigMapAndSecretWithInvalidKeys(t *testing.T) {
 	}
 
 	// Populate the pods's environment.
-	err := populateEnvironmentVariables(context.Background(), pod, rm, er)
+	err := PopulateEnvironmentVariables(context.Background(), pod, rm, er)
 	assert.Check(t, err)
 
 	// Make sure that the container's environment has two variables (corresponding to the single valid key in both the configmap and the secret).
@@ -703,7 +703,7 @@ func TestEnvOverridesEnvFrom(t *testing.T) {
 	}
 
 	// Populate the pods's environment.
-	err := populateEnvironmentVariables(context.Background(), pod, rm, er)
+	err := PopulateEnvironmentVariables(context.Background(), pod, rm, er)
 	assert.Check(t, err)
 
 	// Make sure that the container's environment contains all the expected keys and values.
@@ -780,7 +780,7 @@ func TestEnvFromInexistentConfigMaps(t *testing.T) {
 	}
 
 	// Populate the pods's environment.
-	err := populateEnvironmentVariables(context.Background(), pod, rm, er)
+	err := PopulateEnvironmentVariables(context.Background(), pod, rm, er)
 	assert.Check(t, is.ErrorContains(err, ""))
 
 	// Make sure that two events have been recorded with the correct reason and message.
@@ -837,7 +837,7 @@ func TestEnvFromInexistentSecrets(t *testing.T) {
 	}
 
 	// Populate the pods's environment.
-	err := populateEnvironmentVariables(context.Background(), pod, rm, er)
+	err := PopulateEnvironmentVariables(context.Background(), pod, rm, er)
 	assert.Check(t, is.ErrorContains(err, ""))
 
 	// Make sure that two events have been recorded with the correct reason and message.
@@ -877,7 +877,7 @@ func TestEnvReferencingInexistentConfigMapKey(t *testing.T) {
 									},
 									Key: "key",
 									// This scenario has been observed before https://github.com/virtual-kubelet/virtual-kubelet/issues/444#issuecomment-449611851.
-									// A nil value of optional means "mandatory", hence we should expect "populateEnvironmentVariables" to return an error.
+									// A nil value of optional means "mandatory", hence we should expect "PopulateEnvironmentVariables" to return an error.
 									Optional: nil,
 								},
 							},
@@ -890,7 +890,7 @@ func TestEnvReferencingInexistentConfigMapKey(t *testing.T) {
 	}
 
 	// Populate the pods's environment.
-	err := populateEnvironmentVariables(context.Background(), pod, rm, er)
+	err := PopulateEnvironmentVariables(context.Background(), pod, rm, er)
 	assert.Check(t, is.ErrorContains(err, ""))
 
 	// Make sure that two events have been recorded with the correct reason and message.
@@ -927,7 +927,7 @@ func TestEnvReferencingInexistentSecretKey(t *testing.T) {
 									},
 									Key: "key",
 									// This scenario has been observed before https://github.com/virtual-kubelet/virtual-kubelet/issues/444#issuecomment-449611851.
-									// A nil value of optional means "mandatory", hence we should expect "populateEnvironmentVariables" to return an error.
+									// A nil value of optional means "mandatory", hence we should expect "PopulateEnvironmentVariables" to return an error.
 									Optional: nil,
 								},
 							},
@@ -940,7 +940,7 @@ func TestEnvReferencingInexistentSecretKey(t *testing.T) {
 	}
 
 	// Populate the pods's environment.
-	err := populateEnvironmentVariables(context.Background(), pod, rm, er)
+	err := PopulateEnvironmentVariables(context.Background(), pod, rm, er)
 	assert.Check(t, is.ErrorContains(err, ""))
 
 	// Make sure that two events have been recorded with the correct reason and message.
@@ -1023,7 +1023,7 @@ func TestServiceEnvVar(t *testing.T) {
 	for _, tc := range testCases {
 		pod.Spec.EnableServiceLinks = tc.enableServiceLinks
 
-		err := populateEnvironmentVariables(context.Background(), pod, rm, er)
+		err := PopulateEnvironmentVariables(context.Background(), pod, rm, er)
 		assert.NilError(t, err, "[%s]", tc.name)
 		assert.Check(t, is.DeepEqual(pod.Spec.Containers[0].Env, tc.expectedEnvs, sortOpt))
 	}
@@ -1066,7 +1066,7 @@ func TestComposingEnv(t *testing.T) {
 	}
 
 	// Populate the pods's environment.
-	err := populateEnvironmentVariables(context.Background(), pod, rm, er)
+	err := PopulateEnvironmentVariables(context.Background(), pod, rm, er)
 	assert.Check(t, err)
 
 	// Make sure that the container's environment contains all the expected keys and values.

node/pod.go

@@ -21,6 +21,7 @@ import (
 
 	"github.com/google/go-cmp/cmp"
 	pkgerrors "github.com/pkg/errors"
+	"github.com/virtual-kubelet/virtual-kubelet/internal/podutils"
 	"github.com/virtual-kubelet/virtual-kubelet/log"
 	"github.com/virtual-kubelet/virtual-kubelet/trace"
 	corev1 "k8s.io/api/core/v1"
@@ -68,7 +69,7 @@ func (pc *PodController) createOrUpdatePod(ctx context.Context, pod *corev1.Pod)
 
 	// We do this so we don't mutate the pod from the informer cache
 	pod = pod.DeepCopy()
-	if err := populateEnvironmentVariables(ctx, pod, pc.resourceManager, pc.recorder); err != nil {
+	if err := podutils.PopulateEnvironmentVariables(ctx, pod, pc.resourceManager, pc.recorder); err != nil {
 		span.SetStatus(err)
 		return err
 	}

test/e2e/basic.go

@@ -6,7 +6,7 @@ import (
 	"testing"
 	"time"
 
-	"github.com/virtual-kubelet/virtual-kubelet/node"
+	"github.com/virtual-kubelet/virtual-kubelet/internal/podutils"
 	"gotest.tools/assert"
 	v1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -289,7 +289,7 @@ func (ts *EndToEndTestSuite) TestCreatePodWithOptionalInexistentSecrets(t *testi
 	}
 
 	// Wait for an event concerning the missing secret to be reported on the pod.
-	if err := f.WaitUntilPodEventWithReason(pod, node.ReasonOptionalSecretNotFound); err != nil {
+	if err := f.WaitUntilPodEventWithReason(pod, podutils.ReasonOptionalSecretNotFound); err != nil {
 		t.Fatal(err)
 	}
 
@@ -320,7 +320,7 @@ func (ts *EndToEndTestSuite) TestCreatePodWithMandatoryInexistentSecrets(t *test
 	}()
 
 	// Wait for an event concerning the missing secret to be reported on the pod.
-	if err := f.WaitUntilPodEventWithReason(pod, node.ReasonMandatorySecretNotFound); err != nil {
+	if err := f.WaitUntilPodEventWithReason(pod, podutils.ReasonMandatorySecretNotFound); err != nil {
 		t.Fatal(err)
 	}
 
@@ -356,7 +356,7 @@ func (ts *EndToEndTestSuite) TestCreatePodWithOptionalInexistentConfigMap(t *tes
 	}
 
 	// Wait for an event concerning the missing config map to be reported on the pod.
-	if err := f.WaitUntilPodEventWithReason(pod, node.ReasonOptionalConfigMapNotFound); err != nil {
+	if err := f.WaitUntilPodEventWithReason(pod, podutils.ReasonOptionalConfigMapNotFound); err != nil {
 		t.Fatal(err)
 	}
 
@@ -387,7 +387,7 @@ func (ts *EndToEndTestSuite) TestCreatePodWithMandatoryInexistentConfigMap(t *te
 	}()
 
 	// Wait for an event concerning the missing config map to be reported on the pod.
-	if err := f.WaitUntilPodEventWithReason(pod, node.ReasonMandatoryConfigMapNotFound); err != nil {
+	if err := f.WaitUntilPodEventWithReason(pod, podutils.ReasonMandatoryConfigMapNotFound); err != nil {
 		t.Fatal(err)
 	}