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

Make a few parameters to be optional in windows e2e tests #5976

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

zylxjtu
Copy link
Collaborator

@zylxjtu zylxjtu commented Mar 3, 2025

In this PR, it will:
(1): Make the k8s verison to be configurable
(2): Make the openssh to be optinal in e2e test, this will be helpful for scenerios where openssh will not be able to be installed, but we stil want to test some K8s related scenrios

What type of PR is this?
/kind test

What this PR does / why we need it:
(1): Make the k8s verison to be configurable
(2): Make the openssh to be optinal in e2e test, this will be helpful for scenerios where openssh will not be able to be installed, but we stil want to test some K8s related scenrios

All the configurable parameters can be set through env/pipeline
Which issue(s) this PR fixes:

Fixes #

Requirements:

Special notes for your reviewer:

Release note:

none

In this PR, it will:
(1): Make the k8s verison to be configurable
(2): Make the openssh to be optinal in e2e test, this will be helpful
for scenerios where openssh will not be able to be installed, but we
stil want to test some K8s related scenrios
ValidateFileHasContent(ctx, s, "/k/kubeletstart.ps1", "--container-runtime=remote")
ValidateWindowsProcessHasCliArguments(ctx, s, "kubelet.exe", []string{"--rotate-certificates=true", "--client-ca-file=c:\\k\\ca.crt"})
ValidateCiliumIsNotRunningWindows(ctx, s)
if config.Config.SSHEnabled {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

without openssh, the following test will not be able to pass, but with the testing AKS cluster available, we can add other test suites through pipeline/yaml file

Copy link
Contributor

Choose a reason for hiding this comment

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

For the openssh, I see we're bypassing all the checks on the node. I think I have a way around this, but haven't implemented it yet. Basically, at the moment, to run powershell on a node we:

  • create a container on a different node
  • run bash in that container,
  • scp the powershell script to the node, then
  • ssh to the node and run the script

We could instead

  • create a privlidged powershell container on the node under test
  • run the powershell script there

I'll add a task to my todo list to implement that functionality as it'd be nice to still run the tests when a ssh server isn't running. But it's not a blocker for this PR.

@@ -461,7 +461,7 @@ func baseTemplateLinux(t *testing.T, location string, k8sVersion string, arch st
// this been crafted with a lot of trial and pain, some values are not needed, but it takes a lot of time to figure out which ones.
// and we hope to move on to a different config, so I don't want to invest any more time in this-
func baseTemplateWindows(t *testing.T, location string) *datamodel.NodeBootstrappingConfiguration {
kubernetesVersion := "1.29.9"
kubernetesVersion := config.Config.KubernetsVersion

Choose a reason for hiding this comment

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

This change uplifted the default Kubernetes version from 1.29.9 to 1.30.9. I am fine with this change, but want to call out that change in case folks have concerns

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll ping the Linux team to check it's OK for them. All good from my side tho.

Copy link
Collaborator

Choose a reason for hiding this comment

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

discussed offline, we'd prefer resolving the k8s version from the e2e cluster's control plane as an overridable default, though we should be careful about writing version-dependent tests

Location string `env:"E2E_LOCATION" envDefault:"westus3"`
SIGVersionTagName string `env:"SIG_VERSION_TAG_NAME" envDefault:"branch"`
SIGVersionTagValue string `env:"SIG_VERSION_TAG_VALUE" envDefault:"refs/heads/master"`
SkipTestsWithSKUCapacityIssue bool `env:"SKIP_TESTS_WITH_SKU_CAPACITY_ISSUE"`
SubscriptionID string `env:"SUBSCRIPTION_ID" envDefault:"8ecadfc9-d1a3-4ea4-b844-0d9f87e4d7c8"`
SSHEnabled bool `env:"SSH_ENABLED" envDefault:"true" `
TagsToRun string `env:"TAGS_TO_RUN"`
Copy link
Member

Choose a reason for hiding this comment

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

We have a property SSHEnabled in WindowsProfile . could we use this one?

ValidateFileHasContent(ctx, s, "/k/kubeletstart.ps1", "--container-runtime=remote")
ValidateWindowsProcessHasCliArguments(ctx, s, "kubelet.exe", []string{"--rotate-certificates=true", "--client-ca-file=c:\\k\\ca.crt"})
ValidateCiliumIsNotRunningWindows(ctx, s)
if config.Config.SSHEnabled {
Copy link
Member

Choose a reason for hiding this comment

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

With SSH disabled, I think that you can exec into a system container (e.g. csi pod) to run the check. Otherwise, there is no validation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants