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 migration for dv #455

Merged
merged 1 commit into from
Oct 24, 2024
Merged

add migration for dv #455

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion scripts/installer.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ set -o pipefail
set -e

BUNDLE=$(set -x; kubectl get configmap -n cozy-system cozystack -o 'go-template={{index .data "bundle-name"}}')
VERSION=5
VERSION=6
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Migration script 5 exists but is not executable

The migration script for version 5 exists at scripts/migrations/5 but lacks executable permissions (shown as -rw-r--r--), while all previous migration scripts (1-4) are properly executable. This will cause the migration process to fail since the installer script attempts to execute these files directly.

  • scripts/migrations/5 needs executable permissions to be set using chmod +x
🔗 Analysis chain

Verify migration script existence.

The version bump from 5 to 6 requires a corresponding migration script at scripts/migrations/5 to handle the transition.

Let me try to gather more information about the migration scripts in the repository to ensure we're looking at the correct path.

Let me check one more thing to specifically look for migration scripts in any numbered directories, as the installer.sh suggests they should exist with version numbers.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify migration script exists and is executable
# Expected: File should exist and be executable

if [ -x "scripts/migrations/5" ]; then
    echo "Migration script exists and is executable"
    head -n 5 "scripts/migrations/5" # Show first few lines for context
else
    echo "ERROR: Missing or non-executable migration script"
    exit 1
fi

Length of output: 151

Script:

#!/bin/bash
# Search for migration scripts in the repository
echo "=== Looking for migration-related files ==="
fd -t f "migration" -x ls -l {}

echo -e "\n=== Searching for version-related content in installer.sh ==="
cat scripts/installer.sh

echo -e "\n=== Searching for migration-related patterns in the codebase ==="
rg -l "migration" --type sh

Length of output: 3695

Script:

#!/bin/bash
# Look for numbered migration scripts
echo "=== Looking for numbered migration scripts ==="
fd -t f "^[0-9]+$" scripts/migrations

echo "=== Checking migrations directory structure ==="
ls -la scripts/migrations 2>/dev/null || echo "migrations directory not found"

echo "=== Checking if migrations directory exists ==="
if [ -d "scripts/migrations" ]; then
    echo "Directory exists"
    tree scripts/migrations
else
    echo "Directory does not exist"
fi

Length of output: 1023


run_migrations() {
if ! kubectl get configmap -n cozy-system cozystack-version; then
Expand Down
15 changes: 15 additions & 0 deletions scripts/migrations/5
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#!/bin/sh
# Migration 5 --> 6

# Fix tenant-kubernetes PVCs
kubectl get secret -l kamaji.clastix.io/project=kamaji,kamaji.clastix.io/component=admin-kubeconfig -A --output=go-template='{{ range .items }}{{ printf "%s %s %s %s\n" .metadata.namespace .metadata.name (index .metadata.labels "kamaji.clastix.io/name") (index .data "super-admin.conf") }}{{ end }}' | while read NAMESPACE NAME CLUSTER CONFIGB64; do
config=$(mktemp)
echo "$CONFIGB64" | base64 -d > "$config"
kubectl get pv --kubeconfig="$config" --output=go-template='{{ range .items }}{{ printf "%s\n" .metadata.name }}{{ end }}' | while read PVC; do
(set -x; kubectl label pvc --overwrite -n "$NAMESPACE" "$PVC" "cluster.x-k8s.io/cluster-name=$CLUSTER")
(set -x; kubectl label dv --overwrite -n "$NAMESPACE" "$PVC" "cluster.x-k8s.io/cluster-name=$CLUSTER")
done
rm -f "$config"
done
Comment on lines +4 to +13
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

Add error handling and safety measures.

While the core logic is correct, the script could benefit from additional error handling and safety measures:

  1. Missing error handling for kubectl commands
  2. No validation of base64 decoded content
  3. Temporary file could persist if script fails between creation and cleanup

Consider applying these improvements:

 #!/bin/sh
+set -euo pipefail
+
+cleanup() {
+    [ -f "$config" ] && rm -f "$config"
+}
+trap cleanup EXIT
+
 # Migration 5 --> 6
 
 # Fix tenant-kubernetes PVCs
 kubectl get secret -l kamaji.clastix.io/project=kamaji,kamaji.clastix.io/component=admin-kubeconfig -A --output=go-template='{{ range .items }}{{ printf "%s %s %s %s\n" .metadata.namespace .metadata.name (index .metadata.labels "kamaji.clastix.io/name") (index .data "super-admin.conf") }}{{ end }}' | while read NAMESPACE NAME CLUSTER CONFIGB64; do
+  [ -z "$NAMESPACE" ] || [ -z "$CLUSTER" ] || [ -z "$CONFIGB64" ] && echo "Missing required values" && exit 1
   config=$(mktemp)
-  echo "$CONFIGB64" | base64 -d > "$config"
+  if ! echo "$CONFIGB64" | base64 -d > "$config"; then
+    echo "Failed to decode kubeconfig"
+    exit 1
+  fi
+  if ! kubectl --kubeconfig="$config" cluster-info > /dev/null 2>&1; then
+    echo "Invalid kubeconfig"
+    exit 1
+  fi
   kubectl get pv --kubeconfig="$config" --output=go-template='{{ range .items }}{{ printf "%s\n" .metadata.name }}{{ end }}' | while read PVC; do
-    (set -x; kubectl label pvc --overwrite -n "$NAMESPACE" "$PVC" "cluster.x-k8s.io/cluster-name=$CLUSTER")
-    (set -x; kubectl label dv --overwrite -n "$NAMESPACE" "$PVC" "cluster.x-k8s.io/cluster-name=$CLUSTER")
+    if ! kubectl label pvc --overwrite -n "$NAMESPACE" "$PVC" "cluster.x-k8s.io/cluster-name=$CLUSTER"; then
+      echo "Failed to label PVC $PVC"
+      exit 1
+    fi
+    if ! kubectl label dv --overwrite -n "$NAMESPACE" "$PVC" "cluster.x-k8s.io/cluster-name=$CLUSTER"; then
+      echo "Failed to label DV $PVC"
+      exit 1
+    fi
   done
-  rm -f "$config"
 done
📝 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
# Fix tenant-kubernetes PVCs
kubectl get secret -l kamaji.clastix.io/project=kamaji,kamaji.clastix.io/component=admin-kubeconfig -A --output=go-template='{{ range .items }}{{ printf "%s %s %s %s\n" .metadata.namespace .metadata.name (index .metadata.labels "kamaji.clastix.io/name") (index .data "super-admin.conf") }}{{ end }}' | while read NAMESPACE NAME CLUSTER CONFIGB64; do
config=$(mktemp)
echo "$CONFIGB64" | base64 -d > "$config"
kubectl get pv --kubeconfig="$config" --output=go-template='{{ range .items }}{{ printf "%s\n" .metadata.name }}{{ end }}' | while read PVC; do
(set -x; kubectl label pvc --overwrite -n "$NAMESPACE" "$PVC" "cluster.x-k8s.io/cluster-name=$CLUSTER")
(set -x; kubectl label dv --overwrite -n "$NAMESPACE" "$PVC" "cluster.x-k8s.io/cluster-name=$CLUSTER")
done
rm -f "$config"
done
#!/bin/sh
set -euo pipefail
cleanup() {
[ -f "$config" ] && rm -f "$config"
}
trap cleanup EXIT
# Migration 5 --> 6
# Fix tenant-kubernetes PVCs
kubectl get secret -l kamaji.clastix.io/project=kamaji,kamaji.clastix.io/component=admin-kubeconfig -A --output=go-template='{{ range .items }}{{ printf "%s %s %s %s\n" .metadata.namespace .metadata.name (index .metadata.labels "kamaji.clastix.io/name") (index .data "super-admin.conf") }}{{ end }}' | while read NAMESPACE NAME CLUSTER CONFIGB64; do
[ -z "$NAMESPACE" ] || [ -z "$CLUSTER" ] || [ -z "$CONFIGB64" ] && echo "Missing required values" && exit 1
config=$(mktemp)
if ! echo "$CONFIGB64" | base64 -d > "$config"; then
echo "Failed to decode kubeconfig"
exit 1
fi
if ! kubectl --kubeconfig="$config" cluster-info > /dev/null 2>&1; then
echo "Invalid kubeconfig"
exit 1
fi
kubectl get pv --kubeconfig="$config" --output=go-template='{{ range .items }}{{ printf "%s\n" .metadata.name }}{{ end }}' | while read PVC; do
if ! kubectl label pvc --overwrite -n "$NAMESPACE" "$PVC" "cluster.x-k8s.io/cluster-name=$CLUSTER"; then
echo "Failed to label PVC $PVC"
exit 1
fi
if ! kubectl label dv --overwrite -n "$NAMESPACE" "$PVC" "cluster.x-k8s.io/cluster-name=$CLUSTER"; then
echo "Failed to label DV $PVC"
exit 1
fi
done
done


kubectl create configmap -n cozy-system cozystack-version --from-literal=version=6 --dry-run=client -o yaml | kubectl apply -f-
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

Add error handling for version update.

The version update command should verify success.

Apply this improvement:

-kubectl create configmap -n cozy-system cozystack-version --from-literal=version=6 --dry-run=client -o yaml | kubectl apply -f-
+if ! kubectl create configmap -n cozy-system cozystack-version --from-literal=version=6 --dry-run=client -o yaml | kubectl apply -f-; then
+  echo "Failed to update version ConfigMap"
+  exit 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
kubectl create configmap -n cozy-system cozystack-version --from-literal=version=6 --dry-run=client -o yaml | kubectl apply -f-
if ! kubectl create configmap -n cozy-system cozystack-version --from-literal=version=6 --dry-run=client -o yaml | kubectl apply -f-; then
echo "Failed to update version ConfigMap"
exit 1
fi

Loading