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.

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

e2e_node: add a test to verify kubelet fails to create pod if userns isn't supported #127484

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
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
89 changes: 89 additions & 0 deletions test/e2e_node/user_namespaces_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
//go:build linux
// +build linux

/*
Copyright 2024 The Kubernetes Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package e2enode

import (
"context"
"fmt"
"time"

"github.com/onsi/gomega"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
kubefeatures "k8s.io/kubernetes/pkg/features"
kubeletconfig "k8s.io/kubernetes/pkg/kubelet/apis/config"
"k8s.io/kubernetes/test/e2e/framework"
e2epod "k8s.io/kubernetes/test/e2e/framework/pod"
e2eskipper "k8s.io/kubernetes/test/e2e/framework/skipper"
"k8s.io/kubernetes/test/e2e/nodefeature"
imageutils "k8s.io/kubernetes/test/utils/image"
admissionapi "k8s.io/pod-security-admission/api"
)

var _ = SIGDescribe("UserNamespaces", "[LinuxOnly]", nodefeature.UserNamespacesSupport, framework.WithSerial(), func() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this test really serial?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, when ever a feature gate is toggled for kubelet, the test has to be serial. Changing kubelet configs triggers a restart of kubelet.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Til, thanks, this explains why some test here are tagged as serial when they are not ..., authors may forget to remove the serial decorator when it is ga .... @pohly does it make sense to automate this behavior for the e2e_node so authors don't need to worry about this behavior?

cc @SergeyKanzhelev

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see how. nodefeature.UserNamespacesSupport just says "this test depends on this feature", it doesn't say "this test needs to toogle that feature", so nodefeature.UserNamespacesSupport should not implicitly add the serial tag.

authors may forget to remove the serial decorator when it is ga

Doesn't it say below that the entire test needs to be removed once the feature is GA?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah this test is only serial because it changes the kubelet's feature gates, not specifically toggles user namespaces. Once user namespaces are GA'd (and specifically when the feature gate is dropped), we have to remove this test because we can't test with user namespaces disabled anymore

f := framework.NewDefaultFramework("user-namespace-off-test")
f.NamespacePodSecurityLevel = admissionapi.LevelPrivileged
f.Context("when UserNamespacesSupport=false in the kubelet", func() {
// Turn off UserNamespacesSupport for this test
haircommander marked this conversation as resolved.
Show resolved Hide resolved
// TODO: once the UserNamespacesSupport feature is removed, this test should be removed too
tempSetCurrentKubeletConfig(f, func(ctx context.Context, initialConfig *kubeletconfig.KubeletConfiguration) {
if initialConfig.FeatureGates == nil {
initialConfig.FeatureGates = make(map[string]bool)
}
initialConfig.FeatureGates[string(kubefeatures.UserNamespacesSupport)] = false
})
f.It("will fail to create a hostUsers=false pod", func(ctx context.Context) {
if on, ok := serviceFeatureGates[string(kubefeatures.UserNamespacesSupport)]; !ok || !on {
e2eskipper.Skipf("services do not have user namespaces on")
}
falseVar := false
podClient := e2epod.NewPodClient(f)
pod, err := podClient.PodInterface.Create(ctx, &v1.Pod{
ObjectMeta: metav1.ObjectMeta{Name: "userns-pod"},
Spec: v1.PodSpec{
Containers: []v1.Container{
{
Name: "test-container-1",
Image: imageutils.GetE2EImage(imageutils.BusyBox),
Command: []string{"/bin/sleep"},
Args: []string{"10000"},
},
},
HostUsers: &falseVar,
},
}, metav1.CreateOptions{})
framework.ExpectNoError(err)

// Pod should stay in pending
// Events would be a better way to tell this, as we could actually read the event,
// but history proves events aren't reliable enough to base a test on.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

v1.Events? those must no be used on tests, are best effort , there is no guarantee there are going to be delivered

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exactly

gomega.Consistently(ctx, func() error {
p, err := f.ClientSet.CoreV1().Pods(f.Namespace.Name).Get(ctx, pod.Name, metav1.GetOptions{})
if err != nil {
return err
}
if p.Status.Phase != v1.PodPending {
return fmt.Errorf("Pod phase isn't pending")
}
return nil
}, 30*time.Second, 5*time.Second).ShouldNot(gomega.HaveOccurred())
})
})
})