Skip to content

chore: backup the original ETCD encryption keys#203

Merged
IceCodeNew merged 3 commits intomasterfrom
dr
Sep 17, 2025
Merged

chore: backup the original ETCD encryption keys#203
IceCodeNew merged 3 commits intomasterfrom
dr

Conversation

@IceCodeNew
Copy link
Copy Markdown
Contributor

@IceCodeNew IceCodeNew commented Sep 17, 2025

Summary by CodeRabbit

  • Documentation
    • Added a backup step on the primary control plane before merging the standby key.
    • Moved the standby-key merge to the primary node and updated the example to show a two-key encryption configuration.
    • Renumbered/reordered steps and clarified that the updated file should overwrite replicas.
    • Updated remote update procedure to back up current config, install the new config from a temporary location, then remove temporary files.
    • Broadened Istio prerequisite to require upgrading Istio and its instances before Kubernetes or the product.
    • Improved phrasing for clarity.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Sep 17, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

Adds a primary control-plane backup step, merges a standby encryption key into the primary encryption-provider.conf (now showing two keys), renumbers steps, restricts the apply loop to primary nodes, updates per-node remote install/backup/cleanup steps, and broadens Istio prerequisite wording.

Changes

Cohort / File(s) Summary of changes
Documentation – Global DR encryption workflow
docs/en/install/global_dr.mdx
Added backup of /etc/kubernetes/encryption-provider.conf on primary control plane; introduced a new merge step to combine standby key into the primary config (YAML example now contains two keys with updated secret values); renumbered steps; limited push/apply loop to primary nodes; updated remote update script to create per-node backups, install /tmp/encryption-provider.conf to /etc/kubernetes/encryption-provider.conf, remove /tmp file, and restart kubelet by removing the kube-apiserver pod if present; minor wording adjustments.
Documentation – Pre-upgrade prerequisites
docs/en/upgrade/pre-upgrade.mdx
Broadened Istio prerequisite wording to require upgrading Istio and its instances before upgrading Kubernetes or the product (<Term name="productShort" />) instead of only before Kubernetes.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Admin
  participant PrimaryCP as Primary CP (1.1.1.1)
  participant Primaries as Primary nodes (other primaries)
  participant Node as Target node
  participant Kube as kubelet/kube-apiserver

  Admin->>PrimaryCP: Backup /etc/kubernetes/encryption-provider.conf -> encryption-provider.conf.bak
  Admin->>PrimaryCP: Merge standby key -> updated encryption-provider.conf (key1,key2)

  Admin->>Primaries: Push merged encryption-provider.conf

  loop for each primary node
    Primaries->>Node: cp /etc/kubernetes/encryption-provider.conf /etc/kubernetes/encryption-provider.conf.bak
    Node->>Node: install /tmp/encryption-provider.conf -> /etc/kubernetes/encryption-provider.conf
    Node->>Node: rm /tmp/encryption-provider.conf
    Node->>Kube: remove kube-apiserver pod (if present) to trigger restart
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • chinameok

Poem

A rabbit hops in docs tonight,
Backing keys beneath the moonlight.
Two secrets merged with gentle cheer,
Sent to primaries, kept near.
Kubelets stir — the cluster's right. 🐇🔐

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "chore: backup the original ETCD encryption keys" correctly reflects a real and important change in the diff (adding a backup step for the primary cluster’s encryption-provider.conf), so it is related to the changeset; however the PR also includes merging the standby key, renumbering steps, narrowing the node deployment scope, and updating remote update scripts and wording, which the title does not mention, making it somewhat narrow.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9e174c1 and bbd7aec.

📒 Files selected for processing (1)
  • docs/en/install/global_dr.mdx (3 hunks)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
docs/en/install/global_dr.mdx (1)

335-343: Fix backticks in echo, handle multiple crictl IDs, and avoid hard-failing when none found

Backticks in the quoted echo trigger command substitution; crictl can return multiple IDs — remove them individually and WARN instead of exiting. Replace the block with the diff below.

-    _pod_name="kube-apiserver"
-    _pod_id=$(sudo crictl ps --name "${_pod_name}" --no-trunc --quiet)
-    if [[ -z "${_pod_id}" ]]; then
-        echo "FATAL: could not find pod `kube-apiserver` on node $(hostname)"
-        exit 1
-    fi
-    sudo crictl rm --force "${_pod_id}"
-    sudo systemctl restart kubelet.service
+    _pod_name="kube-apiserver"
+    _pod_ids=$(sudo crictl ps --name "${_pod_name}" --quiet)
+    if [[ -z "${_pod_ids}" ]]; then
+        echo "WARN: kube-apiserver container not found on node $(hostname)"
+    else
+        xargs -r -t -- sudo crictl rm --force <<< "${_pod_ids}"
+    fi
+    sudo systemctl restart kubelet.service

Confirm control-plane uses crictl/containerd (or CRI-O); adjust commands if not.

🧹 Nitpick comments (5)
docs/en/install/global_dr.mdx (5)

297-299: Call out write-key ordering (first key is used for writes).

Make the invariant explicit to prevent accidental write-key flips.

-3. Merge the ETCD encryption key of the standby cluster into the `/etc/kubernetes/encryption-provider.conf` file on node `1.1.1.1`, ensuring the key names are unique.
+3. Merge the ETCD encryption key of the standby cluster into the `/etc/kubernetes/encryption-provider.conf` file on node `1.1.1.1`, ensuring key names are unique.
+   NOTE: Kubernetes uses the first key in the list for writes. Keep the current primary-cluster key first to avoid re-encrypting new data with the standby key.

315-316: Grammar: “every replica”.

Minor wording fix.

-4. Make sure the new `/etc/kubernetes/encryption-provider.conf` file overwrites EVERY replicas on the control plane nodes of both clusters:
+4. Make sure the new `/etc/kubernetes/encryption-provider.conf` file overwrites every replica on the control plane nodes of both clusters:

321-323: Grammar: remove the article before an IP.

-# Since the 1.1.1.1 has already been configured to use both of the ETCD encryption keys,
+# Since 1.1.1.1 has already been configured to use both ETCD encryption keys,

332-334: Deterministic backup perms; ensure target dir exists.

Guarantee 600 perms on the backup and create the directory defensively.

-    sudo /bin/cp -f /etc/kubernetes/encryption-provider.conf /etc/kubernetes/encryption-provider.conf.bak
-    sudo install -o root -g root -m 600 /tmp/encryption-provider.conf /etc/kubernetes/encryption-provider.conf
+    sudo mkdir -p /etc/kubernetes
+    sudo install -o root -g root -m 600 /etc/kubernetes/encryption-provider.conf /etc/kubernetes/encryption-provider.conf.bak
+    sudo install -o root -g root -m 600 /tmp/encryption-provider.conf /etc/kubernetes/encryption-provider.conf
     sudo rm -f /tmp/encryption-provider.conf

347-358: Duplicate step number; same backtick bug here.

Renumber and apply the same robust restart snippet.

-4. Restart the kube-apiserver on node 1.1.1.1
+5. Restart the kube-apiserver on node 1.1.1.1
@@
-    _pod_name="kube-apiserver"
-    _pod_id=$(sudo crictl ps --name "${_pod_name}" --no-trunc --quiet)
-    if [[ -z "${_pod_id}" ]]; then
-        echo "FATAL: could not find pod `kube-apiserver` on node $(hostname)"
-        exit 1
-    fi
-    sudo crictl rm --force "${_pod_id}"
-    sudo systemctl restart kubelet.service
+    _pod_name="kube-apiserver"
+    _pod_ids=$(sudo crictl ps --name "${_pod_name}" --quiet)
+    if [[ -z "${_pod_ids}" ]]; then
+        echo "WARN: kube-apiserver container not found on node $(hostname)"
+    else
+        xargs -r -t -- sudo crictl rm --force <<< "${_pod_ids}"
+    fi
+    sudo systemctl restart kubelet.service
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cf6fd3a and 9637ec5.

📒 Files selected for processing (1)
  • docs/en/install/global_dr.mdx (3 hunks)

Comment thread docs/en/install/global_dr.mdx Outdated
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9637ec5 and 9e174c1.

📒 Files selected for processing (1)
  • docs/en/upgrade/pre-upgrade.mdx (1 hunks)

Comment thread docs/en/upgrade/pre-upgrade.mdx
@IceCodeNew IceCodeNew merged commit acafe1b into master Sep 17, 2025
0 of 2 checks passed
@IceCodeNew IceCodeNew deleted the dr branch September 17, 2025 06:43
IceCodeNew added a commit that referenced this pull request Sep 17, 2025
* chore: backup the original ETCD encryption keys

* chore

* chore: apply the suggestions of CodeRabbit
IceCodeNew added a commit that referenced this pull request Sep 17, 2025
* chore: backup the original ETCD encryption keys

* chore

* chore: apply the suggestions of CodeRabbit
IceCodeNew added a commit that referenced this pull request Sep 17, 2025
* chore: backup the original ETCD encryption keys

* chore

* chore: apply the suggestions of CodeRabbit
IceCodeNew added a commit that referenced this pull request Sep 17, 2025
* chore: backup the original ETCD encryption keys

* chore

* chore: apply the suggestions of CodeRabbit
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.

1 participant