test(robot): add uninstallation check test case #2069

chriscchien · 2024-08-23T08:28:26Z

Which issue(s) this PR fixes:

Issue #longhorn/longhorn#9222

What this PR does / why we need it:

Transfer manual test case Uninstallation Checks into robot framework

Special notes for your reviewer:

Having environment below variable set first if run on local

LONGHORN_INSTALL_METHOD (manifest or helm)
LONGHORN_REPO_BRANCH
CUSTOM_LONGHORN_MANAGER_IMAGE (if not using master-head)
CUSTOM_LONGHORN_ENGINE_IMAGE (if not using master-head)
CUSTOM_LONGHORN_INSTANCE_MANAGER_IMAGE (if not using master-head)
CUSTOM_LONGHORN_SHARE_MANAGER_IMAGE (if not using master-head)
-CUSTOM_LONGHORN_BACKING_IMAGE_MANAGER_IMAGE (if not using master-head)

issue command

`./run.sh -t "Uninstallation Checks"

Additional documentation or context

report.zip

chriscchien · 2024-08-26T01:45:17Z

Need to wait for clarification on longhorn/longhorn#9303 before reviewing this PR. Currently, the PR only checks logs for level=error and level=fatal instead of looking for the word error alone.

assert "level=error" not in logs
assert "level=fatal" not in logs

chriscchien · 2024-08-26T09:31:29Z

Need to wait for clarification on longhorn/longhorn#9303 before reviewing this PR. Currently, the PR only checks logs for level=error and level=fatal instead of looking for the word error alone.
assert "level=error" not in logs
assert "level=fatal" not in logs

This ticket is ready for review, I have changed the log check part to make sure no word error appeared in uninstall pod.

yangchiu · 2024-08-27T06:57:15Z

e2e/tests/test_cases/uninstallation_checks.robot

+    ...    4. Check all the CRDs are removed kubectl get crds | grep longhorn.
+    ...    
+    ...    Not implemented Steps
+    ...    5. Check the backup stores, the backups taken should NOT be removed.


If some steps are still unimplemented, doesn't that mean this test case still requires manually execution? We can implement the install longhorn keyword to re-install Longhorn again and use the same backup store, which would allow steps 5 and 6 to be automated. WDYT?

Updated, but does step 6 can automated? because test case mentioned the DR volume is in another cluster.

Yes. Since you uninstall and reinstall Longhorn, the reinstalled Longhorn is another cluster.

yangchiu · 2024-08-27T06:57:30Z

e2e/tests/test_cases/uninstallation_checks.robot

+${RETRY_COUNT}    300
+${RETRY_INTERVAL}    1
+${DATA_ENGINE}    v1
+${LONGHORN_BRANCH}    masater


yangchiu · 2024-08-27T07:04:59Z

e2e/libs/uninstallation/uninstallation.py

+
+import os
+
+class Uninstallation:


The Uninstallation class is too specific. We can create a Longhorn abstract class includes install, uninstall, upgrade and other related interfaces. Then, classes like LonghornKubectl, LonghornHelmChart, LonghornRancherChart can extend the Longhorn class to implement these interfaces.

Updated, we already have e2e/libs/longhorn.py so I add below in e2e/libs/longhorn_deploy
.
├── base.py
├── init.py
├── longhorn_deploy.py
├── longhorn_helm_chart.py
└── longhorn_kubectl.py

yangchiu · 2024-08-27T07:10:19Z

e2e/keywords/longhorn.resource

@@ -43,3 +44,9 @@ Check Longhorn workload pods ${condition} annotated with ${key}
    Run Keyword IF    '${condition}' == 'not'    Should Not Be True    ${is_annotated}
    ...    ELSE IF    '${condition}' == 'is'    Should Be True    ${is_annotated}
    ...    ELSE    Fail    Invalid condition ${condition}
+
+Uninstall Longhorn ${LONGHORN_BRANCH} by ${INSTALL_METHOD}


We can use the INSTALL_METHOD environment variable to determine which class to instantiate, eliminating the need to explicitly specify Uninstall Longhorn by XXX. WDYT?

In libs/longhorn_deploy/lognhorn.py added below part because Jenkins did not have LONGHORN_INSTALL_METHOD on e2e pipeline, if run on local and using helm, we need to set set LONGHORN_INSTALL_METHOD to helm first

_method = os.getenv("LONGHORN_INSTALL_METHOD", "manifest")

yangchiu · 2024-09-04T02:56:54Z

e2e/libs/keywords/uninstallation_keywords.py

@@ -0,0 +1,12 @@
+from uninstallation import Uninstallation


Is file uninstallation_keywords.py still needed?

yangchiu · 2024-09-04T03:00:43Z

e2e/tests/test_cases/uninstallation_checks.robot

+    ...    4. Check all the CRDs are removed kubectl get crds | grep longhorn.
+    ...    
+    ...    Not implemented Steps
+    ...    5. Check the backup stores, the backups taken should NOT be removed.


Yes. Since you uninstall and reinstall Longhorn, the reinstalled Longhorn is another cluster.

yangchiu · 2024-09-04T03:12:37Z

e2e/tests/test_cases/uninstallation_checks.robot

+${RETRY_COUNT}    300
+${RETRY_INTERVAL}    1
+${DATA_ENGINE}    v1
+${LONGHORN_BRANCH}    master


It would be ideal if the installation parameters, such as LONGHORN_REPO_BRANCH, CUSTOM_LONGHORN_MANAGER_IMAGE, CUSTOM_LONGHORN_ENGINE_IMAGE, could be retrieved from Jenkins environment variables, and passed to a shell script for installing Longhorn using different method, similar to what our test_upgrade pytest and upgrade_longhorn.sh do. Do you think it's possible for you to implement this?

Updated, added e2e/utilities/longhorn-setup.sh to get environment variable to customize test images. I tested on local and Jenkins

report.zip

yangchiu · 2024-09-04T03:14:12Z

e2e/libs/uninstallation/uninstallation.py

@@ -0,0 +1,57 @@
+from utility.utility import logging


Is file uninstallation.py still needed?

mergify · 2024-09-06T02:50:17Z

This pull request is now in conflict. Could you fix it @chriscchien? 🙏

yangchiu · 2024-09-23T10:20:44Z

e2e/utilities/longhorn-setup.sh

Why is adding e2e/utilities/longhorn-setup.sh necessary to obtain the environment variables for custom images? Aren't those variables already passed to the container in pipeline/e2e/Jenkinsfile at https://github.com/longhorn/longhorn-tests/blob/master/pipelines/e2e/Jenkinsfile#L95?

This test case will uninstall LH and then reinstall again. The reinstall Longhorn steps in e2e/utilities/longhorn-setup.sh is executed in robot framework which is running in longhorn-test pod (run_longhorn_e2e_test.sh), thus I need pass the environments when create the test pod, otherwise the reinstall will fail because information for custom images are None.
If this doesn't fully answer the question, could you elaborate more on your concern? thanks.

In addition, by current implementation, some Jenkins env variable did't pass into instacne :

ip-10-0-1-121:/home/ec2-user # printenv LS_COLORS=no=00:fi=00:di=01;34:ln=00;36:pi=40;33:so=01;35:do=01;35:bd=40;33;01:cd=40;33;01:or=41;33;01:ex=00;32:*.cmd=00;32:*.exe=01;32:*.com=01;32:*.bat=01;32:*.btm=01;32:*.dll=01;32:*.tar=00;31:*.tbz=00;31:*.tgz=00;31:*.rpm=00;31:*.deb=00;31:*.arj=00;31:*.taz=00;31:*.lzh=00;31:*.lzma=00;31:*.zip=00;31:*.zoo=00;31:*.z=00;31:*.Z=00;31:*.gz=00;31:*.bz2=00;31:*.tb2=00;31:*.tz2=00;31:*.tbz2=00;31:*.xz=00;31:*.avi=01;35:*.bmp=01;35:*.dl=01;35:*.fli=01;35:*.gif=01;35:*.gl=01;35:*.jpg=01;35:*.jpeg=01;35:*.mkv=01;35:*.mng=01;35:*.mov=01;35:*.mp4=01;35:*.mpg=01;35:*.pcx=01;35:*.pbm=01;35:*.pgm=01;35:*.png=01;35:*.ppm=01;35:*.svg=01;35:*.tga=01;35:*.tif=01;35:*.webm=01;35:*.webp=01;35:*.wmv=01;35:*.xbm=01;35:*.xcf=01;35:*.xpm=01;35:*.aiff=00;32:*.ape=00;32:*.au=00;32:*.flac=00;32:*.m4a=00;32:*.mid=00;32:*.mp3=00;32:*.mpc=00;32:*.ogg=00;32:*.voc=00;32:*.wav=00;32:*.wma=00;32:*.wv=00;32: HOSTTYPE=x86_64 LESSCLOSE=lessclose.sh %s %s XKEYSYMDB=/usr/X11R6/lib/X11/XKeysymDB LANG=POSIX WINDOWMANAGER=xterm LESS=-M -I -R SUDO_GID=100 HOSTNAME=ip-10-0-1-121 CSHEDIT=emacs GPG_TTY=/dev/pts/2 LESS_ADVANCED_PREPROCESSOR=no COLORTERM=1 SUDO_COMMAND=/usr/bin/su MACHTYPE=x86_64-suse-linux MINICOM=-c on NCURSES_NO_UTF8_ACS=1 OSTYPE=linux USER=root PAGER=less MORE=-sl PWD=/home/ec2-user HOME=/root LC_CTYPE=C.UTF-8 HOST=ip-10-0-1-121 SUDO_USER=ec2-user XNLSPATH=/usr/X11R6/lib/X11/nls XDG_DATA_DIRS=/usr/share PROFILEREAD=true SUDO_UID=1000 FROM_HEADER= MAIL=/var/mail/root LESSKEY=/etc/lesskey.bin SHELL=/bin/bash TERM=xterm-256color LS_OPTIONS=-A -N --color=tty -T 0 SHLVL=1 MANPATH=/usr/share/man:/usr/local/man LOGNAME=root XDG_CONFIG_DIRS=/etc/xdg PATH=/sbin:/usr/sbin:/usr/local/sbin:/root/bin:/usr/local/bin:/usr/bin:/bin:/usr/lib/mit/bin G_BROKEN_FILENAMES=1 HISTSIZE=1000 CPU=x86_64 LESSOPEN=lessopen.sh %s _=/usr/bin/printenv

Is it possible to reuse functions from e2e/utilities/ instead of copying and pasting the entire longhorn-setup.sh file?

Also, can we pass custom component image versions as parameters (e.g., -v CUSTOM_LONGHORN_INSTANCE_MANAGER_IMAGE:longhornio/longhorn-instance-manager:master-head) similar to what we did in test_upgrade pytest. so we don't have to modify the test.yaml file?

@yangchiu , I have updated this PR, refined e2e/utilities/ to source other script files instead of copy the code, also this PR tested on local and Jenkins.

currently on Jenkins e2e it support manifest install only

on Local, it support reinstall longhorn by helm by export LONGHORN_INSTALL_METHOD=helm

chriscchien · 2024-10-01T01:13:24Z

@longhorn/qa , this ticket updated and ready for revirew, thank you.

mergify · 2024-10-01T01:13:50Z

This pull request is now in conflict. Could you fix it @chriscchien? 🙏

yangchiu · 2024-10-01T07:56:11Z

pipelines/utilities/run_longhorn_e2e_test.sh

@@ -31,6 +32,30 @@ run_longhorn_e2e_test(){
  fi

  yq e -i 'select(.spec.containers[0] != null).spec.containers[0].env[6].value="'${LONGHORN_TEST_CLOUDPROVIDER}'"' ${LONGHORN_TESTS_MANIFEST_FILE_PATH}
+  yq e -i 'select(.spec.containers[0].env != null).spec.containers[0].env += {"name": "HOST_PROVIDER", "value": "'${LONGHORN_TEST_CLOUDPROVIDER}'"}' "${LONGHORN_TESTS_MANIFEST_FILE_PATH}"


This part should be removed as it's based on outdated content.

yangchiu · 2024-10-01T07:59:14Z

e2e/utilities/longhorn-install.sh

+
+set -x
+
+source ../test_framework/scripts/longhorn-setup.sh


Is it possible to source files only from the pipelines/utilities folder?

updated

source ../pipelines/utilities/longhorn_ui.sh source ../pipelines/utilities/create_longhorn_namespace.sh source ../pipelines/utilities/install_backupstores.sh source ../pipelines/utilities/longhorn_status.sh source ../pipelines/utilities/longhorn_helm_chart.sh source ../pipelines/utilities/create_aws_secret.sh source ../pipelines/utilities/longhorn_manifest.sh

yangchiu · 2024-10-01T08:09:23Z

e2e/libs/k8s/k8s.py

+            target_namespace = api.read_namespace(name=namespace)
+            target_namespace_status = target_namespace.status.phase
+            logging(f"Waiting for namespace {target_namespace.metadata.name} terminated, current status is {target_namespace_status} retry ({i}) ...")
+        except ApiException:


Should it also verify if e.status == 404?

updated

try: target_namespace = api.read_namespace(name=namespace) target_namespace_status = target_namespace.status.phase logging(f"Waiting for namespace {target_namespace.metadata.name} terminated, current status is {target_namespace_status} retry ({i}) ...") except ApiException as e: if e.status == 404: logging(f"Namespace {namespace} successfully terminated.") return else: logging(f"Error while fetching namespace {namespace} status: {e}")

yangchiu · 2024-10-01T08:17:33Z

pipelines/utilities/run_longhorn_e2e_test.sh

+  yq e -i 'select(.spec.containers[0].env != null).spec.containers[0].env += {"name": "LONGHORN_INSTALL_METHOD", "value": "'${LONGHORN_INSTALL_METHOD}'"}' "${LONGHORN_TESTS_MANIFEST_FILE_PATH}"
+
+  set +x
+  if [[ "${LONGHORN_TEST_CLOUDPROVIDER}" == "aws" ]]; then


This part should be removed as it's based on outdated content.

mergify · 2024-10-01T08:43:26Z

This pull request is now in conflict. Could you fix it @chriscchien? 🙏

chriscchien · 2024-10-01T15:30:37Z

Hi @yangchiu , updated and tested passsed on local(manifest & helm), Jenkins(manifest), thank you.

mergify · 2024-10-05T00:51:53Z

This pull request is now in conflict. Could you fix it @chriscchien? 🙏

yangchiu · 2024-10-08T03:45:31Z

e2e/utilities/longhorn-install.sh

+
+LONGHORN_NAMESPACE="longhorn-system"
+
+install_longhorn_by_chart(){


Can we reuse functions in longhorn_helm_chart.sh here?

Hi @yangchiu, Updated!

ref: longhorn/longhorn#9222 Signed-off-by: Chris <chris.chien@suse.com>

yangchiu

LGTM

chriscchien self-assigned this Aug 23, 2024

chriscchien requested a review from a team as a code owner August 23, 2024 08:28

chriscchien force-pushed the uninstall-robot branch from a92eacc to 00a132e Compare August 23, 2024 08:30

chriscchien removed the request for review from a team August 26, 2024 01:45

chriscchien force-pushed the uninstall-robot branch from 00a132e to 8a51ea4 Compare August 26, 2024 09:23

chriscchien requested a review from a team August 26, 2024 09:31

yangchiu requested changes Aug 27, 2024

View reviewed changes

chriscchien force-pushed the uninstall-robot branch from bd5f4aa to 2104d09 Compare August 29, 2024 03:38

chriscchien requested review from yangchiu and removed request for yangchiu August 29, 2024 03:45

chriscchien closed this Aug 29, 2024

chriscchien reopened this Sep 3, 2024

chriscchien force-pushed the uninstall-robot branch from 2104d09 to 1e468df Compare September 3, 2024 10:45

chriscchien requested a review from yangchiu September 3, 2024 10:45

chriscchien force-pushed the uninstall-robot branch from 1e468df to 7bea403 Compare September 3, 2024 10:50

yangchiu requested changes Sep 4, 2024

View reviewed changes

chriscchien force-pushed the uninstall-robot branch 8 times, most recently from c800a05 to 55282d8 Compare September 4, 2024 09:48

chriscchien force-pushed the uninstall-robot branch 3 times, most recently from 21d5f33 to 6148528 Compare September 11, 2024 10:02

yangchiu requested changes Sep 23, 2024

View reviewed changes

chriscchien requested a review from yangchiu September 25, 2024 03:41

chriscchien force-pushed the uninstall-robot branch 3 times, most recently from a873602 to 97cb50c Compare September 27, 2024 05:13

chriscchien requested a review from a team October 1, 2024 01:13

chriscchien force-pushed the uninstall-robot branch from 97cb50c to ff2607e Compare October 1, 2024 01:52

yangchiu requested changes Oct 1, 2024

View reviewed changes

chriscchien force-pushed the uninstall-robot branch 9 times, most recently from 642417d to 73bee6c Compare October 1, 2024 15:02

chriscchien requested a review from yangchiu October 1, 2024 15:27

yangchiu requested changes Oct 8, 2024

View reviewed changes

chriscchien force-pushed the uninstall-robot branch from 73bee6c to a769d84 Compare October 8, 2024 05:34

test(robot): add uninstallation check test case

2f9abb2

ref: longhorn/longhorn#9222 Signed-off-by: Chris <chris.chien@suse.com>

chriscchien force-pushed the uninstall-robot branch from 34aedd9 to 2f9abb2 Compare October 8, 2024 06:10

chriscchien requested a review from yangchiu October 8, 2024 06:36

yangchiu approved these changes Oct 8, 2024

View reviewed changes

yangchiu merged commit 56c64a9 into longhorn:master Oct 8, 2024
7 of 8 checks passed


		LONGHORN_NAMESPACE="longhorn-system"

		install_longhorn_by_chart(){

test(robot): add uninstallation check test case #2069

test(robot): add uninstallation check test case #2069

Conversation

chriscchien commented Aug 23, 2024 • edited Loading

Which issue(s) this PR fixes:

What this PR does / why we need it:

Special notes for your reviewer:

Additional documentation or context

chriscchien commented Aug 26, 2024

chriscchien commented Aug 26, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chriscchien Sep 13, 2024 • edited Loading

Choose a reason for hiding this comment

yangchiu Sep 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yangchiu Sep 4, 2024 • edited Loading

Choose a reason for hiding this comment

mergify bot commented Sep 6, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chriscchien Sep 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chriscchien commented Oct 1, 2024

mergify bot commented Oct 1, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mergify bot commented Oct 1, 2024

chriscchien commented Oct 1, 2024

mergify bot commented Oct 5, 2024

Choose a reason for hiding this comment

chriscchien Oct 8, 2024 • edited Loading

Choose a reason for hiding this comment

yangchiu left a comment

Choose a reason for hiding this comment

chriscchien commented Aug 23, 2024 •

edited

Loading

chriscchien Sep 13, 2024 •

edited

Loading

yangchiu Sep 4, 2024 •

edited

Loading

yangchiu Sep 4, 2024 •

edited

Loading

chriscchien Sep 25, 2024 •

edited

Loading

chriscchien Oct 8, 2024 •

edited

Loading