Add Turbinia e2e GKE test #1400

wajihyassine · 2023-11-22T19:49:02Z

Description of the change

Updates Turbinia GCP/GKE e2e testing using new Turbinia client and API server instead.

Checklist

All tests were successful.

hacktobeer

Good to see we are moving the e2e tests to GKE as well! PTAL.

turbinia/e2e/run-e2e-gke.sh

hacktobeer · 2023-11-29T13:29:33Z

turbinia/e2e/run-e2e-gke.sh

+# The e2e test will test Turbinia googleclouddisk processing
+# Requirements:
+# - have 'kubectl', 'jq' packages installed
+# - have the Turbinia Helm chart deployed and are authenticated to the GKE cluster


add command needed to authenticate? or maybe even do this in the script as you already need zone/projectname anyway.

gcloud container clusters get-credentials [clustername] --zone [zone] --project [project_name]

Ack I added it to the requirements for now

turbinia/e2e/run-e2e-gke.sh

hacktobeer · 2023-11-29T13:33:46Z

turbinia/e2e/run-e2e-gke.sh

+echo -n "Started at "
+date -Iseconds
+
+# Back up existing Turbinia config else script will attempt to connect to wrong Turbinia instance


Both backing up and replacing config? Would be nice to restore this at the end of the script.

put functionality in bash functions in script (backup, replace, restore)

call them to backup/replace and at the end to restore original configuration.

Ack added a way to restore the backup at the end. holding off on moving things into functions because then I'll want to move everything into functions which could be a nice cleanup but something we can do in like a V2

Or we could leave this copy alone and then write the new copy to a different directory (e.g. something like ~/.turbinia-e2e-test/`) and point to it with the command line flag.

actually like Aarons suggestion. that is much cleaner.

hacktobeer · 2023-11-29T13:34:23Z

turbinia/e2e/run-e2e-gke.sh

+fi
+
+# Replace Turbinia config with test config
+echo "Writing turbinia config to $HOME/.turbinia_api_config.json..."


You use $HOME here and ~/ above, I would use one or the other.

Ack good catch updated it to just use ~

turbinia/e2e/run-e2e-gke.sh

hacktobeer · 2023-11-29T13:38:44Z

turbinia/e2e/run-e2e-gke.sh

+echo "Running Turbinia: turbinia-client submit googleclouddisk --project $GCP_PROJECT --zone $GCP_ZONE --disk_name $DISK --request_id $REQUEST_ID"
+turbinia-client submit googleclouddisk --project $GCP_PROJECT --zone $GCP_ZONE --disk_name $DISK --request_id $REQUEST_ID
+# Give time for Tasks to populate
+sleep 5


You are waiting here until the request has reached status 'running' I think. Why not use the same method as below to check that the request is in running. Once it is you can continue.

I updated the while statement slightly to evaluate on while $req_status = "running".

The other reason I did this is because the time you submit the request has lag time to when its actually reached the server and is stored in redis so I'm sleeping here to wait a little, maybe a better way to do this so open to ideas. I think if we had polling implemented directly then this wouldn't be much of an issue

hacktobeer · 2023-11-29T13:39:56Z

turbinia/e2e/run-e2e-gke.sh

+done
+
+# Grab all Tasks where successful = false
+echo "Checking the status of Turbinia request: $REQUEST_ID"


I think we need a feature request to have the turbinia client be able to filter status requests on eg failed tasks ( or other relevant status filters).

Yeah that would be neat for filtering through the request and can use it for Web UI as well so you don't have to subtract and filter like this. Lmk if you want to file a bug else I can tmrw

hacktobeer · 2023-11-29T13:41:19Z

turbinia/e2e/run-e2e-gke.sh

+# Check if there is a failed Turbinia Task
+if [[ $length > 0 ]]
+then
+	echo "A failed Task for Turbinia Request $req has been detected."


some indentation issues in below 'if' and further 'for/do' loops

Yeah weird it didnt show in my vscode but updated to fix the formatting

turbinia/e2e/run-e2e-gke.sh

wajihyassine

Thank you @hacktobeer for the thorough look! Everything should be addressed and ready for another pass

wajihyassine · 2023-11-30T00:08:49Z

turbinia/e2e/run-e2e-gke.sh

+# The e2e test will test Turbinia googleclouddisk processing
+# Requirements:
+# - have 'kubectl', 'jq' packages installed
+# - have the Turbinia Helm chart deployed and are authenticated to the GKE cluster


Ack I added it to the requirements for now

turbinia/e2e/run-e2e-gke.sh

wajihyassine · 2023-11-30T00:09:50Z

turbinia/e2e/run-e2e-gke.sh

+echo -n "Started at "
+date -Iseconds
+
+# Back up existing Turbinia config else script will attempt to connect to wrong Turbinia instance


Ack added a way to restore the backup at the end. holding off on moving things into functions because then I'll want to move everything into functions which could be a nice cleanup but something we can do in like a V2

wajihyassine · 2023-11-30T00:10:06Z

turbinia/e2e/run-e2e-gke.sh

+fi
+
+# Replace Turbinia config with test config
+echo "Writing turbinia config to $HOME/.turbinia_api_config.json..."


Ack good catch updated it to just use ~

turbinia/e2e/run-e2e-gke.sh

wajihyassine · 2023-11-30T00:17:34Z

turbinia/e2e/run-e2e-gke.sh

+echo "Running Turbinia: turbinia-client submit googleclouddisk --project $GCP_PROJECT --zone $GCP_ZONE --disk_name $DISK --request_id $REQUEST_ID"
+turbinia-client submit googleclouddisk --project $GCP_PROJECT --zone $GCP_ZONE --disk_name $DISK --request_id $REQUEST_ID
+# Give time for Tasks to populate
+sleep 5


I updated the while statement slightly to evaluate on while $req_status = "running".

The other reason I did this is because the time you submit the request has lag time to when its actually reached the server and is stored in redis so I'm sleeping here to wait a little, maybe a better way to do this so open to ideas. I think if we had polling implemented directly then this wouldn't be much of an issue

wajihyassine · 2023-11-30T00:17:56Z

turbinia/e2e/run-e2e-gke.sh

+# Check if there is a failed Turbinia Task
+if [[ $length > 0 ]]
+then
+	echo "A failed Task for Turbinia Request $req has been detected."


Yeah weird it didnt show in my vscode but updated to fix the formatting

turbinia/e2e/run-e2e-gke.sh

wajihyassine · 2023-11-30T00:28:09Z

turbinia/e2e/run-e2e-gke.sh

+done
+
+# Grab all Tasks where successful = false
+echo "Checking the status of Turbinia request: $REQUEST_ID"


Yeah that would be neat for filtering through the request and can use it for Web UI as well so you don't have to subtract and filter like this. Lmk if you want to file a bug else I can tmrw

aarontp

From a high level and cursory look it LGTM and since @hacktobeer has already reviewed I think we should be set from the review standpoint. Thanks!

aarontp · 2023-11-30T00:47:26Z

turbinia/e2e/run-e2e-gke.sh

+echo -n "Started at "
+date -Iseconds
+
+# Back up existing Turbinia config else script will attempt to connect to wrong Turbinia instance


Or we could leave this copy alone and then write the new copy to a different directory (e.g. something like ~/.turbinia-e2e-test/`) and point to it with the command line flag.

hacktobeer

Have a look at comment Aaron. Makes sense to use a different location for the config file and use it with the client CLI switch.
For the rest -> Approved.

wajihyassine · 2023-11-30T20:54:43Z

Awesome thank you for the review @hacktobeer and @aarontp!

Talking to Aaron out of band, it looks like the -c param is broken and will file a bug separately and leaving the config logic is fine as is for now given will just be used for the automated e2e tests but can be updated in a V2 iteration later

* Add Turbinia e2e GKE test * rename e2e script * list turbinia config sleep on port-forward * fix spelling * Address review comments

wajihyassine added 4 commits November 22, 2023 19:48

Add Turbinia e2e GKE test

b559115

rename e2e script

d906f30

list turbinia config sleep on port-forward

cc9ae52

fix spelling

0a3b41b

wajihyassine self-assigned this Nov 22, 2023

wajihyassine marked this pull request as ready for review November 22, 2023 22:02

wajihyassine requested review from aarontp and hacktobeer November 22, 2023 22:02

hacktobeer requested changes Nov 29, 2023

View reviewed changes

Address review comments

199a774

wajihyassine commented Nov 30, 2023

View reviewed changes

aarontp reviewed Nov 30, 2023

View reviewed changes

hacktobeer approved these changes Nov 30, 2023

View reviewed changes

wajihyassine merged commit 5881b67 into master Nov 30, 2023

wajihyassine mentioned this pull request Dec 18, 2023

Add integration test script google/osdfir-infrastructure#88

Merged

jleaniz pushed a commit to jleaniz/turbinia that referenced this pull request Feb 28, 2024

Add Turbinia e2e GKE test (google#1400)

36c48a9

* Add Turbinia e2e GKE test * rename e2e script * list turbinia config sleep on port-forward * fix spelling * Address review comments

jleaniz pushed a commit to jleaniz/turbinia that referenced this pull request Mar 18, 2024

Add Turbinia e2e GKE test (google#1400)

87480d5

* Add Turbinia e2e GKE test * rename e2e script * list turbinia config sleep on port-forward * fix spelling * Address review comments

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Turbinia e2e GKE test #1400

Add Turbinia e2e GKE test #1400

wajihyassine commented Nov 22, 2023 •

edited

Loading

hacktobeer left a comment

hacktobeer Nov 29, 2023

wajihyassine Nov 30, 2023

hacktobeer Nov 29, 2023

wajihyassine Nov 30, 2023

aarontp Nov 30, 2023

hacktobeer Nov 30, 2023

hacktobeer Nov 29, 2023

wajihyassine Nov 30, 2023

hacktobeer Nov 29, 2023

wajihyassine Nov 30, 2023

hacktobeer Nov 29, 2023

wajihyassine Nov 30, 2023

hacktobeer Nov 29, 2023

wajihyassine Nov 30, 2023

wajihyassine left a comment

wajihyassine Nov 30, 2023

wajihyassine Nov 30, 2023

wajihyassine Nov 30, 2023

wajihyassine Nov 30, 2023

wajihyassine Nov 30, 2023

wajihyassine Nov 30, 2023

aarontp left a comment

aarontp Nov 30, 2023

hacktobeer left a comment

wajihyassine commented Nov 30, 2023

Add Turbinia e2e GKE test #1400

Add Turbinia e2e GKE test #1400

Conversation

wajihyassine commented Nov 22, 2023 • edited Loading

Description of the change

Checklist

hacktobeer left a 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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wajihyassine left a 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

Choose a reason for hiding this comment

aarontp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hacktobeer left a comment

Choose a reason for hiding this comment

wajihyassine commented Nov 30, 2023

wajihyassine commented Nov 22, 2023 •

edited

Loading