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.

Sign up for GitHub

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

Add tests refactor #451

Merged
merged 6 commits into from
Oct 22, 2024
+170 −9
Merged

Add tests refactor #451

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3bf136f
add e2e for apps
klinch0 Oct 15, 2024
4512e10
Refactor e2e tests for applications
kvaps Oct 18, 2024
8251a22
fix tests
klinch0 Oct 22, 2024
f409573
fix app tests
klinch0 Oct 22, 2024
d640ef1
fix install yq jq
klinch0 Oct 22, 2024
95897d6
refactor
kvaps Oct 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions .pre-commit-config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,14 @@ repos:
- repo: https://github.com/igorshubovych/markdownlint-cli
rev: v0.41.0
hooks:
- id: markdownlint
args: [--fix, --disable, MD013, MD041, --]
- id: markdownlint
args: [--fix, --disable, MD013, MD041, --]
- repo: local
hooks:
- id: gen-versions-map
name: Generate versions map and check for changes
entry: bash -c 'cd packages/apps && make check-version-map'
language: system
types: [file]
pass_filenames: false
description: Run the script and fail if it generates changes
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance the new gen-versions-map hook for robustness and efficiency.

The addition of the gen-versions-map hook is a good practice for maintaining version consistency. However, consider the following improvements:

  1. Limit the hook to specific file types or paths to improve efficiency.
  2. Add error handling for the cd command and the make execution.
  3. Consider using always_run: true instead of types: [file] if this should run regardless of file changes.

Here's a suggested improvement:

- repo: local
  hooks:
    - id: gen-versions-map
      name: Generate versions map and check for changes
      entry: bash -c 'cd packages/apps && make check-version-map || exit 1'
      language: system
      always_run: true
      pass_filenames: false
      description: Run the script and fail if it generates changes or encounters an error

This change ensures the hook runs on every commit, handles errors properly, and provides clearer feedback.

135 changes: 135 additions & 0 deletions hack/e2e.application.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
#!/bin/bash

RED='\033[0;31m'
GREEN='\033[0;32m'
RESET='\033[0m'
YELLOW='\033[0;33m'


ROOT_NS="tenant-root"
TEST_TENANT="tenant-e2e"

function clean() {
kubectl delete helmrelease.helm.toolkit.fluxcd.io $TEST_TENANT -n $ROOT_NS
if true; then
echo -e "${GREEN}Cleanup successful!${RESET}"
return 0
else
echo -e "${RED}Cleanup failed!${RESET}"
return 1
fi
}
Comment on lines +12 to +21
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Improve error handling in the clean function.

The current implementation always reports success, regardless of whether the deletion was actually successful. This could lead to false positives.

Consider modifying the function to actually check the deletion status:

 function clean() {
-    kubectl delete helmrelease.helm.toolkit.fluxcd.io $TEST_TENANT -n $ROOT_NS
-    if true; then
+    if kubectl delete helmrelease.helm.toolkit.fluxcd.io $TEST_TENANT -n $ROOT_NS; then
         echo -e "${GREEN}Cleanup successful!${RESET}"
         return 0
     else
         echo -e "${RED}Cleanup failed!${RESET}"
         return 1
     fi
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
function clean() {
kubectl delete helmrelease.helm.toolkit.fluxcd.io $TEST_TENANT -n $ROOT_NS
if true; then
echo -e "${GREEN}Cleanup successful!${RESET}"
return 0
else
echo -e "${RED}Cleanup failed!${RESET}"
return 1
fi
}
function clean() {
if kubectl delete helmrelease.helm.toolkit.fluxcd.io $TEST_TENANT -n $ROOT_NS; then
echo -e "${GREEN}Cleanup successful!${RESET}"
return 0
else
echo -e "${RED}Cleanup failed!${RESET}"
return 1
fi
}


function install_helmrelease() {
local release_name="$1"
local namespace="$2"
local chart_path="$3"
local repo_name="$4"
local repo_ns="$5"
local values_file="$6"

if [[ -z "$release_name" ]]; then
echo -e "${RED}Error: Release name is required.${RESET}"
exit 1
fi

if [[ -z "$namespace" ]]; then
echo -e "${RED}Error: Namespace name is required.${RESET}"
exit 1
fi

if [[ -z "$chart_path" ]]; then
echo -e "${RED}Error: Chart path name is required.${RESET}"
exit 1
fi

local helmrelease_file=$(mktemp /tmp/HelmRelease.XXXXXX.yaml)
{
echo "apiVersion: helm.toolkit.fluxcd.io/v2"
echo "kind: HelmRelease"
echo "metadata:"
echo " labels:"
echo " cozystack.io/ui: \"true\""
echo " name: \"$release_name\""
echo " namespace: \"$namespace\""
echo "spec:"
echo " chart:"
echo " spec:"
echo " chart: \"$chart_path\""
echo " reconcileStrategy: Revision"
echo " sourceRef:"
echo " kind: HelmRepository"
echo " name: \"$repo_name\""
echo " namespace: \"$repo_ns\""
echo " version: '*'"
echo " interval: 1m0s"
echo " timeout: 5m0s"

if [[ -n "$values_file" && -f "$values_file" ]]; then
echo " values:"
cat "$values_file" | sed 's/^/ /'
fi
} > "$helmrelease_file"

kubectl apply -f "$helmrelease_file"

rm -f "$helmrelease_file"
}
Comment on lines +23 to +77
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Improve error handling in install_helmrelease function.

The function is well-structured, but there's room for improvement in error handling.

  1. Address the Shellcheck warning by separating the declaration and assignment of helmrelease_file:
-    local helmrelease_file=$(mktemp /tmp/HelmRelease.XXXXXX.yaml)
+    local helmrelease_file
+    helmrelease_file=$(mktemp /tmp/HelmRelease.XXXXXX.yaml) || { echo "Failed to create temp file"; exit 1; }
  1. Add error handling for the kubectl apply command:
-    kubectl apply -f "$helmrelease_file"
+    if ! kubectl apply -f "$helmrelease_file"; then
+        echo -e "${RED}Failed to apply HelmRelease${RESET}"
+        rm -f "$helmrelease_file"
+        return 1
+    fi

These changes will make the function more robust and prevent potential issues from silent failures.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
function install_helmrelease() {
local release_name="$1"
local namespace="$2"
local chart_path="$3"
local repo_name="$4"
local repo_ns="$5"
local values_file="$6"
if [[ -z "$release_name" ]]; then
echo -e "${RED}Error: Release name is required.${RESET}"
exit 1
fi
if [[ -z "$namespace" ]]; then
echo -e "${RED}Error: Namespace name is required.${RESET}"
exit 1
fi
if [[ -z "$chart_path" ]]; then
echo -e "${RED}Error: Chart path name is required.${RESET}"
exit 1
fi
local helmrelease_file=$(mktemp /tmp/HelmRelease.XXXXXX.yaml)
{
echo "apiVersion: helm.toolkit.fluxcd.io/v2"
echo "kind: HelmRelease"
echo "metadata:"
echo " labels:"
echo " cozystack.io/ui: \"true\""
echo " name: \"$release_name\""
echo " namespace: \"$namespace\""
echo "spec:"
echo " chart:"
echo " spec:"
echo " chart: \"$chart_path\""
echo " reconcileStrategy: Revision"
echo " sourceRef:"
echo " kind: HelmRepository"
echo " name: \"$repo_name\""
echo " namespace: \"$repo_ns\""
echo " version: '*'"
echo " interval: 1m0s"
echo " timeout: 5m0s"
if [[ -n "$values_file" && -f "$values_file" ]]; then
echo " values:"
cat "$values_file" | sed 's/^/ /'
fi
} > "$helmrelease_file"
kubectl apply -f "$helmrelease_file"
rm -f "$helmrelease_file"
}
function install_helmrelease() {
local release_name="$1"
local namespace="$2"
local chart_path="$3"
local repo_name="$4"
local repo_ns="$5"
local values_file="$6"
if [[ -z "$release_name" ]]; then
echo -e "${RED}Error: Release name is required.${RESET}"
exit 1
fi
if [[ -z "$namespace" ]]; then
echo -e "${RED}Error: Namespace name is required.${RESET}"
exit 1
fi
if [[ -z "$chart_path" ]]; then
echo -e "${RED}Error: Chart path name is required.${RESET}"
exit 1
fi
local helmrelease_file
helmrelease_file=$(mktemp /tmp/HelmRelease.XXXXXX.yaml) || { echo "Failed to create temp file"; exit 1; }
{
echo "apiVersion: helm.toolkit.fluxcd.io/v2"
echo "kind: HelmRelease"
echo "metadata:"
echo " labels:"
echo " cozystack.io/ui: \"true\""
echo " name: \"$release_name\""
echo " namespace: \"$namespace\""
echo "spec:"
echo " chart:"
echo " spec:"
echo " chart: \"$chart_path\""
echo " reconcileStrategy: Revision"
echo " sourceRef:"
echo " kind: HelmRepository"
echo " name: \"$repo_name\""
echo " namespace: \"$repo_ns\""
echo " version: '*'"
echo " interval: 1m0s"
echo " timeout: 5m0s"
if [[ -n "$values_file" && -f "$values_file" ]]; then
echo " values:"
cat "$values_file" | sed 's/^/ /'
fi
} > "$helmrelease_file"
if ! kubectl apply -f "$helmrelease_file"; then
echo -e "${RED}Failed to apply HelmRelease${RESET}"
rm -f "$helmrelease_file"
return 1
fi
rm -f "$helmrelease_file"
}
🧰 Tools
🪛 Shellcheck

[warning] 46-46: Declare and assign separately to avoid masking return values.

(SC2155)


function install_tenant (){
local release_name="$1"
local namespace="$2"
local values_file="${3:-tenant.yaml}"
local repo_name="cozystack-apps"
local repo_ns="cozy-public"

install_helmrelease "$release_name" "$namespace" "tenant" "$repo_name" "$repo_ns" "$values_file"
}

function check_helmrelease_status() {
local release_name="$1"
local namespace="$2"
local timeout=300 # Timeout in seconds
local interval=5 # Interval between checks in seconds
local elapsed=0

while [[ $elapsed -lt $timeout ]]; do
local status_output
status_output=$(kubectl get helmrelease "$release_name" -n "$namespace" -o json | jq -r '.status.conditions[-1].reason')

if [[ "$status_output" == "InstallSucceeded" ]]; then
echo -e "${GREEN}Helm release '$release_name' is ready.${RESET}"
return 0
elif [[ "$status_output" == "InstallFailed" ]]; then
echo -e "${RED}Helm release '$release_name': InstallFailed${RESET}"
exit 1
else
echo -e "${YELLOW}Helm release '$release_name' is not ready. Current status: $status_output${RESET}"
fi

sleep "$interval"
elapsed=$((elapsed + interval))
done

echo -e "${RED}Timeout reached. Helm release '$release_name' is still not ready after $timeout seconds.${RESET}"
exit 1
}

chart_name="$1"

if [ -z "$chart_name" ]; then
echo -e "${RED}No chart name provided. Exiting...${RESET}"
exit 1
fi

echo "Running tests for chart: $chart_name"
install_tenant $TEST_TENANT $ROOT_NS
check_helmrelease_status $TEST_TENANT $ROOT_NS

repo_name="cozystack-apps"
repo_ns="cozy-public"

release_name="$chart_name-e2e"
install_helmrelease "$release_name" "$TEST_TENANT" "$chart_name" "$repo_name" "$repo_ns"

check_helmrelease_status "$release_name" "$TEST_TENANT"
6 changes: 6 additions & 0 deletions hack/values/tenant.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
host: ""
etcd: false
monitoring: true
ingress: false
seaweedfs: true
isolated: true
4 changes: 2 additions & 2 deletions packages/apps/ferretdb/templates/init-script.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ stringData:
DROP USER $user;
EOT
done

echo "== create roles"
psql -v ON_ERROR_STOP=1 --echo-all <<\EOT
SELECT 'CREATE ROLE app_admin NOINHERIT;'
Expand All @@ -83,7 +83,7 @@ stringData:
FOR schema_record IN SELECT schema_name FROM information_schema.schemata WHERE schema_name NOT IN ('pg_catalog', 'information_schema') LOOP
-- Changing Schema Ownership
EXECUTE format('ALTER SCHEMA %I OWNER TO %I', schema_record.schema_name, 'app_admin');

-- Add rights for the admin role
EXECUTE format('GRANT ALL ON SCHEMA %I TO %I', schema_record.schema_name, 'app_admin');
EXECUTE format('GRANT ALL ON ALL TABLES IN SCHEMA %I TO %I', schema_record.schema_name, 'app_admin');
Expand Down
Empty file modified packages/core/testing/Chart.yaml
100644 → 100755
View file
Open in desktop
Empty file.
15 changes: 14 additions & 1 deletion packages/core/testing/Makefile
100644 → 100755
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
NAMESPACE=cozy-e2e-tests
NAME := sandbox
CLEAN := 1
TESTING_APPS := $(shell find ../../apps -maxdepth 1 -mindepth 1 -type d | awk -F/ '{print $$NF}')

include ../../../scripts/common-envs.mk

Expand Down Expand Up @@ -32,11 +33,23 @@ image-e2e-sandbox:
yq -i '.e2e.image = strenv(IMAGE)' values.yaml
rm -f images/e2e-sandbox.json

copy-hack-dir:
tar -C ../../../ -cf- hack | kubectl exec -i -n $(NAMESPACE) deploy/cozystack-e2e-$(NAME) -- tar -xf-
Comment on lines +36 to +37
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider using kubectl cp to simplify directory copying

Using kubectl cp can streamline the process of copying the hack directory into the Kubernetes deployment, making the command more readable and maintainable.

Apply this diff to use kubectl cp:

copy-hack-dir:
-	tar -C ../../../ -cf- hack | kubectl exec -i -n $(NAMESPACE) deploy/cozystack-e2e-$(NAME) -- tar -xf-
+	kubectl cp ../../../hack $(NAMESPACE)/deploy/cozystack-e2e-$(NAME):/hack

Note: Ensure that the kubectl cp command works correctly in your environment and that the pod has the necessary permissions to receive files.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
copy-hack-dir:
tar -C ../../../ -cf- hack | kubectl exec -i -n $(NAMESPACE) deploy/cozystack-e2e-$(NAME) -- tar -xf-
copy-hack-dir:
kubectl cp ../../../hack $(NAMESPACE)/deploy/cozystack-e2e-$(NAME):/hack


test: wait-for-sandbox ## Run the end-to-end tests in existing sandbox.
cat ../../../hack/e2e.sh | kubectl exec -i -n $(NAMESPACE) deploy/cozystack-e2e-$(NAME) -- sh -c 'cat > /e2e.sh && chmod +x /e2e.sh'
helm template -n cozy-system installer ../installer | kubectl exec -i -n $(NAMESPACE) deploy/cozystack-e2e-$(NAME) -- sh -c 'cat > /cozystack-installer.yaml'
kubectl exec -ti -n $(NAMESPACE) deploy/cozystack-e2e-$(NAME) -- sh -c 'export COZYSTACK_INSTALLER_YAML=$$(cat /cozystack-installer.yaml) && /e2e.sh'

test-applications: wait-for-sandbox copy-hack-dir ## Run the end-to-end tests in existing sandbox for applications.
kubectl exec -ti -n $(NAMESPACE) deploy/cozystack-e2e-$(NAME) -- sh -c 'wget https://github.com/mikefarah/yq/releases/download/v4.44.3/yq_linux_amd64 -O /usr/local/bin/yq > /dev/null 2>&1 && chmod +x /usr/local/bin/yq && apt-get install jq -y > /dev/null 2>&1'
for app in $(TESTING_APPS); do \
echo "Running tests for $${app}"; \
kubectl exec -ti -n cozy-e2e-tests deploy/cozystack-e2e-sandbox -- bash -c "/hack/e2e.application.sh $${app}"; \
done
kubectl exec -ti -n cozy-e2e-tests deploy/cozystack-e2e-sandbox -- bash -c "kubectl get hr -A | grep -v 'True'"

test-result: wait-for-sandbox copy-hack-dir ## Run the end-to-end tests in existing sandbox for applications.

delete: ## Remove sandbox from existing Kubernetes cluster.
kubectl delete deploy -n $(NAMESPACE) cozystack-e2e-$(NAME)

Expand Down
Empty file modified packages/core/testing/images/e2e-sandbox/Dockerfile
100644 → 100755
View file
Open in desktop
Empty file.
Empty file modified packages/core/testing/templates/sandbox.yaml
100644 → 100755
View file
Open in desktop
Empty file.
Empty file modified packages/core/testing/values.yaml
100644 → 100755
View file
Open in desktop
Empty file.
Loading