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
refactor: migrate node e2e tests off insecure port #94723
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
|
@@ -18,18 +18,17 @@ package services | |||||||
|
||||||||
import ( | ||||||||
"fmt" | ||||||||
"io/ioutil" | ||||||||
"net" | ||||||||
|
||||||||
"k8s.io/apiserver/pkg/storage/storagebackend" | ||||||||
|
||||||||
apiserver "k8s.io/kubernetes/cmd/kube-apiserver/app" | ||||||||
"k8s.io/kubernetes/cmd/kube-apiserver/app/options" | ||||||||
"k8s.io/kubernetes/test/e2e/framework" | ||||||||
) | ||||||||
|
||||||||
const ( | ||||||||
clusterIPRange = "10.0.0.1/24" | ||||||||
apiserverClientURL = "http://localhost:8080" | ||||||||
apiserverHealthCheckURL = apiserverClientURL + "/healthz" | ||||||||
) | ||||||||
const clusterIPRange = "10.0.0.1/24" | ||||||||
|
||||||||
// APIServer is a server which manages apiserver. | ||||||||
type APIServer struct { | ||||||||
|
@@ -47,14 +46,23 @@ func NewAPIServer(storageConfig storagebackend.Config) *APIServer { | |||||||
|
||||||||
// Start starts the apiserver, returns when apiserver is ready. | ||||||||
func (a *APIServer) Start() error { | ||||||||
const tokenFilePath = "known_tokens.csv" | ||||||||
|
||||||||
o := options.NewServerRunOptions() | ||||||||
o.Etcd.StorageConfig = a.storageConfig | ||||||||
_, ipnet, err := net.ParseCIDR(clusterIPRange) | ||||||||
if err != nil { | ||||||||
return err | ||||||||
} | ||||||||
o.SecureServing.BindAddress = net.ParseIP("127.0.0.1") | ||||||||
// Disable insecure serving | ||||||||
o.InsecureServing.BindPort = 0 | ||||||||
o.ServiceClusterIPRanges = ipnet.String() | ||||||||
o.AllowPrivileged = true | ||||||||
if err := generateTokenFile(tokenFilePath); err != nil { | ||||||||
return fmt.Errorf("failed to generate token file %s: %v", tokenFilePath, err) | ||||||||
} | ||||||||
o.Authentication.TokenFile.TokenFile = tokenFilePath | ||||||||
o.Admission.GenericAdmission.DisablePlugins = []string{"ServiceAccount", "TaintNodesByCondition"} | ||||||||
errCh := make(chan error) | ||||||||
go func() { | ||||||||
|
@@ -71,7 +79,7 @@ func (a *APIServer) Start() error { | |||||||
} | ||||||||
}() | ||||||||
|
||||||||
err = readinessCheck("apiserver", []string{apiserverHealthCheckURL}, errCh) | ||||||||
err = readinessCheck("apiserver", []string{getAPIServerHealthCheckURL()}, errCh) | ||||||||
if err != nil { | ||||||||
return err | ||||||||
} | ||||||||
|
@@ -96,9 +104,14 @@ func (a *APIServer) Name() string { | |||||||
} | ||||||||
|
||||||||
func getAPIServerClientURL() string { | ||||||||
return apiserverClientURL | ||||||||
return framework.TestContext.Host | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What does this get set to? Because you're hardcoding the bind address to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh I forgot the listen address is hard-coded.. then do we need to enforce There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe we should just default the TestContext.Host to localhost? (only for the node suite though) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
kubernetes/test/e2e/framework/test_context.go Lines 419 to 421 in 7a2812c
|
||||||||
} | ||||||||
|
||||||||
func getAPIServerHealthCheckURL() string { | ||||||||
return apiserverHealthCheckURL | ||||||||
return framework.TestContext.Host + "/healthz" | ||||||||
} | ||||||||
|
||||||||
func generateTokenFile(tokenFilePath string) error { | ||||||||
tokenFile := fmt.Sprintf("%s,kubelet,uid,system:masters\n", framework.TestContext.BearerToken) | ||||||||
knight42 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
return ioutil.WriteFile(tokenFilePath, []byte(tokenFile), 0644) | ||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,12 +27,12 @@ import ( | |
"time" | ||
|
||
"github.com/spf13/pflag" | ||
"k8s.io/klog/v2" | ||
|
||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
utilfeature "k8s.io/apiserver/pkg/util/feature" | ||
cliflag "k8s.io/component-base/cli/flag" | ||
"k8s.io/klog/v2" | ||
kubeletconfigv1beta1 "k8s.io/kubelet/config/v1beta1" | ||
|
||
"k8s.io/kubernetes/cmd/kubelet/app/options" | ||
"k8s.io/kubernetes/pkg/features" | ||
kubeletconfig "k8s.io/kubernetes/pkg/kubelet/apis/config" | ||
|
@@ -356,21 +356,23 @@ func createPodDirectory() (string, error) { | |
|
||
// createKubeconfig creates a kubeconfig file at the fully qualified `path`. The parent dirs must exist. | ||
func createKubeconfig(path string) error { | ||
kubeconfig := []byte(`apiVersion: v1 | ||
kubeconfig := []byte(fmt.Sprintf(`apiVersion: v1 | ||
kind: Config | ||
users: | ||
- name: kubelet | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does the username here need to match the user name in the server-side token file? |
||
user: | ||
token: %s | ||
clusters: | ||
- cluster: | ||
server: ` + getAPIServerClientURL() + ` | ||
server: %s | ||
insecure-skip-tls-verify: true | ||
name: local | ||
contexts: | ||
- context: | ||
cluster: local | ||
user: kubelet | ||
name: local-context | ||
current-context: local-context`) | ||
current-context: local-context`, framework.TestContext.BearerToken, getAPIServerClientURL())) | ||
|
||
if err := ioutil.WriteFile(path, kubeconfig, 0666); err != nil { | ||
return err | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,9 +24,9 @@ import ( | |
"path" | ||
"testing" | ||
|
||
utilfeature "k8s.io/apiserver/pkg/util/feature" | ||
"k8s.io/klog/v2" | ||
|
||
utilfeature "k8s.io/apiserver/pkg/util/feature" | ||
"k8s.io/kubernetes/test/e2e/framework" | ||
) | ||
|
||
|
@@ -65,14 +65,16 @@ func NewE2EServices(monitorParent bool) *E2EServices { | |
func (e *E2EServices) Start() error { | ||
var err error | ||
if !framework.TestContext.NodeConformance { | ||
if e.services, err = e.startInternalServices(); err != nil { | ||
return fmt.Errorf("failed to start internal services: %v", err) | ||
} | ||
// Start kubelet | ||
e.kubelet, err = e.startKubelet() | ||
if err != nil { | ||
return fmt.Errorf("failed to start kubelet: %v", err) | ||
} | ||
} | ||
e.services, err = e.startInternalServices() | ||
return err | ||
return nil | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think our problem for #95877, we don't even start the services in conformance mode now. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps this was moved because we start the kubelet here in conformance tests? So I guess we need to separate starting the internal services. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ya, that's why the kubelet start is inside. But there isn't any change to node_conformance to start the services. |
||
} | ||
|
||
// Stop stops the e2e services. | ||
|
@@ -129,7 +131,11 @@ func (e *E2EServices) startInternalServices() (*server, error) { | |
return nil, fmt.Errorf("can't get current binary: %v", err) | ||
} | ||
// Pass all flags into the child process, so that it will see the same flag set. | ||
startCmd := exec.Command(testBin, append([]string{"--run-services-mode"}, os.Args[1:]...)...) | ||
startCmd := exec.Command(testBin, | ||
append( | ||
[]string{"--run-services-mode", fmt.Sprintf("--bearer-token=%s", framework.TestContext.BearerToken)}, | ||
os.Args[1:]..., | ||
)...) | ||
server := newServer("services", startCmd, nil, nil, getServicesHealthCheckURLs(), servicesLogFile, e.monitorParent, false) | ||
return server, server.start() | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this seems fine as per the docs for SynchronizedBeforeSuite
https://github.com/onsi/ginkgo/blob/master/ginkgo_dsl.go#L504
just to double check my understanding here, does this mean that the rest of framework.TestContext is empty when the child processes are run and only the token field is set?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nope,
framework.TestContext
is set like the parent process and the token field is another random token.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, i see. so this persists the original token that we generated in the parent process.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah you are right.