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

Install logic switched to ele-testhelpers #191

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
Open

Install logic switched to ele-testhelpers #191

wants to merge 17 commits into from

Conversation

thehejik
Copy link
Collaborator

@thehejik thehejik commented Oct 24, 2024

Fixes #189

Totally work in progress - the suite and install files were borrowed from turtles-e2e - needs to be adapted for HP.

RANCHER_PASSWORD=mypwd \
RANCHER_HOSTNAME=1.2.3.4.nip.io \
RANCHER_VERSION=latest/2.9.2 \
INSTALL_K3S_VERSION=v1.28.10+k3s1 \
make prepare-rancher

Todo:

  • create a normal user??? - NOT USED
  • install helm - already in Makefile as install-helm target - now always installs latest
  • install nightly operators - so far kept in Makefile moved to install_suite.go
  • waiting for rancher-webhook pods already implemented, fixes Flake: Fix rancher-webhook missing for test cases #183
  • proxy - run squid in docker, config k3s, certmanager and rancher
  • process squid logs
  • combine nighly x proxy x normal - no big change - only that all scenarios are driven by env variables NIGHTLY_CHART and RANCHER_BEHIND_PROXY (use when != "") and calling make prepare-rancher
  • code cleanup

@thehejik thehejik marked this pull request as draft October 24, 2024 16:06
@thehejik thehejik self-assigned this Oct 24, 2024
@thehejik
Copy link
Collaborator Author

thehejik commented Nov 11, 2024

Notes for initial proxy support:

  • add support for proxy in k3s, cert-manager and rancher via command:
RANCHER_BEHIND_PROXY=anything \
RANCHER_PASSWORD=mypasswd \
RANCHER_HOSTNAME=1.2.3.4.nip.io \
RANCHER_VERSION=latest/devel/2.9 \
INSTALL_K3S_VERSION=v1.28.10+k3s1 \
make prepare-rancher
  • proxyUrl is hardcoded
  • Workflows not modified
  • Cleanup required

@thehejik
Copy link
Collaborator Author

required PR rancher-sandbox/ele-testhelpers#67

  * initial cleanup
  * workflows not adapted yet
  * testhelpers not merged/bumped

Signed-off-by: Tomas Hehejik <[email protected]>
@thehejik
Copy link
Collaborator Author

thehejik commented Nov 13, 2024

Notes for previous commit

  • function complete
  • nightly charts supported (needs change in testhelpers)
  • workflows not adapted yet
  • initial cleanup

Used command to deploy rancher with proxy and with nightly hosted providers chart:

PROVIDER=aks \
NIGHTLY=yes \
RANCHER_BEHIND_PROXY=yes \
RANCHER_PASSWORD=mypassword \
RANCHER_HOSTNAME=1.2.3.4.nip.io \
RANCHER_VERSION=latest/devel/2.10 \
INSTALL_K3S_VERSION=v1.30.6+k3s1 \
make prepare-e2e-ci-rancher-hosted-nightly-chart-behind-proxy

Verified values helm get values rancher -n cattle-system -o yaml:

bootstrapPassword: mypassword
extraEnv:
- name: CATTLE_SERVER_URL
  value: https://1.2.3.4.nip.io
- name: CATTLE_AGENT_IMAGE
  value: rancher/rancher-agent:v2.10-head
- name: CATTLE_SKIP_HOSTED_CLUSTER_CHART_INSTALLATION
  value: "true"
hostname: 1.2.3.4.nip.io
noProxy: 127.0.0.0/8,10.0.0.0/8,cattle-system.svc,172.16.0.0/12,192.168.0.0/16,.svc,.cluster.local
proxy: http://172.17.0.1:3128
rancherImageTag: v2.10-head
replicas: 1
useBundledSystemChart: true

  * ele-testhelpers bumped to support extraFlags for rancher
  * main.yaml adapted
  * proxyHost not hardcoded anymore but using fallback when no set
  * Makefile cleanup and env vat check added for rancher installation
  * still contains debug info

Signed-off-by: Tomas Hehejik <[email protected]>
@thehejik
Copy link
Collaborator Author

First testrun, enabled proxy and nightly on AKS with p0_provisioning:
https://github.com/rancher/hosted-providers-e2e/actions/runs/11841053192

@thehejik
Copy link
Collaborator Author

After fix for installing nightly_charts:
https://github.com/rancher/hosted-providers-e2e/actions/runs/11841322069

@thehejik thehejik changed the title WIP Install logic switched to ele-testhelpers Install logic switched to ele-testhelpers Nov 15, 2024
@thehejik thehejik marked this pull request as ready for review November 15, 2024 10:39
Copy link
Collaborator

@valaparthvi valaparthvi left a comment

Choose a reason for hiding this comment

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

  1. Perhaps hosted/preparation can be moved out of hosted and into the root directory since it is not directly related to testing hosted providers.
  2. On main.yaml L#341 Collect logs step, can you modify it to run if steps.prepare-cluster.outcome == 'success', even if the job is canceled? I recently came across a case where I need to cancel a long running job, but I still wanted to see the logs from the tests that ran.

Overall great work on this, thank you! 🚀

@@ -0,0 +1,82 @@
#
Copy link
Collaborator

Choose a reason for hiding this comment

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

There is a squid.conf in .github/scripts. Can you verify the contents of this file with it and delete it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I made a copy of the original file in hosted/preparation/squid.conf because the original location was harder to access , after moving the code to root dir I can access it easily again. I reverted back to original location.

Comment on lines 68 to 70
if rancherHostname == "" {
Fail("RANCHER_HOSTNAME environment variable is required")
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit:

Suggested change
if rancherHostname == "" {
Fail("RANCHER_HOSTNAME environment variable is required")
}
Expect(rancherHostname).ToNot(BeEmpty(), "RANCHER_HOSTNAME environment variable is required")
}

if rancherHostname == "" {
Fail("RANCHER_HOSTNAME environment variable is required")
}
rancherVersion = os.Getenv("RANCHER_VERSION")
Copy link
Collaborator

Choose a reason for hiding this comment

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

REQUIRED_VARS := INSTALL_K3S_VERSION RANCHER_HOSTNAME RANCHER_PASSWORD RANCHER_VERSION

rancherVersion is a required env var, we should add a Expect(<field>).ToNot(BeEmpty()) check same for the other fields as well. WDYT?

Comment on lines 80 to 94
// Extract Rancher Manager channel/version to install
if rancherVersion != "" {
// Split rancherVersion and reset it
s := strings.Split(rancherVersion, "/")
rancherVersion = ""

// Get needed informations
rancherChannel = s[0]
if len(s) > 1 {
rancherVersion = s[1]
}
if len(s) > 2 {
rancherHeadVersion = s[2]
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

If RANCHER_VERSION is a required field, and assuming we have already checked that it is not empty, we do not need to recheck here.

Suggested change
// Extract Rancher Manager channel/version to install
if rancherVersion != "" {
// Split rancherVersion and reset it
s := strings.Split(rancherVersion, "/")
rancherVersion = ""
// Get needed informations
rancherChannel = s[0]
if len(s) > 1 {
rancherVersion = s[1]
}
if len(s) > 2 {
rancherHeadVersion = s[2]
}
}
// Extract Rancher Manager channel/version to install
// Split rancherVersion and reset it
s := strings.Split(rancherVersion, "/")
Expect(len(s)).To(BeNumerically(">=", 2))
rancherVersion = ""
// Get needed information
rancherChannel = s[0]
rancherVersion = s[1]
if len(s) > 2 {
rancherHeadVersion = s[2]
}
latest/latest, latest/2.9.3[-rc2], prime/2.9.3, prime/devel/2.9, prime-optimus/2.9.3-rc2

Also, if it is provided, on splitting the string, its length would be >= 2, you would need to have the channel and version.
prime/devel/2.9 -> version=devel headVersion=2.9, prime-optimus/2.9.3-rc2 -> version=2.9.3-rc2

fi
elif [ ${{ inputs.proxy }} == true ]; then
make prepare-e2e-ci-rancher-behind-proxy
export NIGHTLY_CHART="enabled"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we use a boolean value for this instead of a string? Same for proxy env.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For nightly it would be possible to use boolean but it's problematic for proxy because testhelpers function for installing Rancher accepts string anyway when requesting proxy. I would rather keep both as strings for now.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Also I removed the else part as it is not actually needed.


By("Installing K3s", func() {
// Get K3s installation script
fileName := "k3s-install.sh"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you add defer to delete this file once the test finishes?

Comment on lines 75 to 77
installCmd := exec.Command("sh", fileName)
// INSTALL_K3S_VERSION should be already set by the action or user's shell
installCmd.Env = append(os.Environ(), "INSTALL_K3S_EXEC=--disable metrics-server --write-kubeconfig-mode 644")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you think there is a way to install k3s without downloading the file?

curl -sfL https://get.k3s.io | INSTALL_K3S_VERSION=$INSTALL_K3S_VERSION sh -s - server --cluster-init --tls-san=$RANCHER_HOSTNAME

I usually use the above command, it installs k3s without downloading the script as far as I know.

GinkgoWriter.Printf("K3s installation loop %d:\n%s\n", count, out)
count++
return err
}, tools.SetTimeout(2*time.Minute), 5*time.Second).Should(BeNil())
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit:
Add a failure message to Eventually. Also add it to other eventually blocks if you go ahead with the suggestion.

Suggested change
}, tools.SetTimeout(2*time.Minute), 5*time.Second).Should(BeNil())
}, tools.SetTimeout(2*time.Minute), 5*time.Second).Should(BeNil(), "Timed out while waiting for k3s to install")

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

AFAIK Should is not designed to return a custom message on failure :(

Copy link
Collaborator

Choose a reason for hiding this comment

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

It does. We are using it at a couple of places within our repo. Search for "Timed out", you'll find it. Let me know if I am mistaken 😓 .


By("Waiting for K3s to be started", func() {
// Wait for all pods to be started
checkList := [][]string{
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think somewhere around here you need to ensure a KUBECONFIG env var is exported. When I tried running this locally without a KUBECONFIG exported, it errored out.

Perhaps, you can add KUBECONFIG to the REQUIRED_VARS list and also ensure it is .ToNot(BeEmpty()) in the BeforeSuite?

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.

Replace Makefile code in targets for installing rancher by r/ele-testhelpers
2 participants