d8dfun/virtual-kubelet
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 1 Wiki Activity

Move env var code into its own package

Browse Source 
This creates a new package -- podutils. The env var related code
doesn't really have any business being part of the node package,
and to create a separation of concerns, faster tests, and just
general code isolation and cleanliness, we can move the env
var related code into this package. This change is purely hygiene,
and not logic related.

For node, the package is under internal, because the constructor
references manager, which is an internal package.
This commit is contained in:
Sargun Dhillon
2020-10-24 12:07:27 -07:00
parent 621ee4bb1c
commit c437e05ad0
4 changed files with 24 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
node/env.go → internal/podutils/env.go
View File

@@ -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.
// 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 {
// PopulateEnvironmentVariables populates the environment of each container (and init container) in the specified pod.
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 {

28
node/env_internal_test.go → internal/podutils/env_internal_test.go
View File

@@ -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.

3
node/pod.go
View File

@@ -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
}

10
test/e2e/basic.go
View File

@@ -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)
}