Migrate to using custom resource to store etcd snapshot metadata #8064

brandond · 2023-07-28T00:24:53Z

Proposed Changes

Add ADR and implementation for etcd snapshot CRD migration.

Types of Changes

ADR, bugfix, reimplementation

Verification

Run full snapshot create / delete / prune tests; confirm that all function normally and snapshot CRs, configmaps, and CLI output stay in sync

Testing

n/a

Linked Issues

k3s-etcd-snapshots configmap grows too large when snapshot retention is high #8611
[BUG] rke2-etcd-snapshots configmap grows too large when snapshot retention is high rancher/rke2#4495

User-Facing Change

Further Comments

manuelbuil

Thanks for writing this up Brad. I have two (stupid?) questions:

1 - Will this impact how rke2 is doing snapshots too?
2 - IIRC, snapshots are stored in /var/lib/rancher/k3s/server/db/snapshots, right? So, what the configmap stores is information about the snapshots stored in the filesystem, right?

docs/adrs/etcd-snapshot-cr.md

brandond · 2023-07-28T17:05:39Z

@manuelbuil

1 - Will this impact how rke2 is doing snapshots too?

Yes, this code manages snapshots for both products.

2 - IIRC, snapshots are stored in /var/lib/rancher/k3s/server/db/snapshots, right? So, what the configmap stores is information about the snapshots stored in the filesystem, right?

Yes, and the snapshots stored in s3.

cwayne18 · 2023-08-14T14:21:04Z

@Oats87 I know we talked this over in design, but it would be good to get sign-off from the v2prov side to make sure y'all are in agreement with whatever we decide to do here

brandond · 2023-08-14T18:39:47Z

I have a couple changes to make based on what was discussed in the call. This includes:

Storing metadata alongside the snapshots both on disk and in s3 so that it doesn't only live in the snapshot list
Use of kube-system namespace UID as cluster ID (to prevent listing/pruning of snapshots from different clusters in the same bucket)

codecov · 2023-09-28T01:25:27Z

Codecov Report

Attention: 932 lines in your changes are missing coverage. Please review.

Comparison is base (7d38b4a) 47.05% compared to head (4854dbf) 50.66%.

❗ Current head 4854dbf differs from pull request most recent head 174ff79. Consider uploading reports for the commit 174ff79 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #8064      +/-   ##
==========================================
+ Coverage   47.05%   50.66%   +3.60%     
==========================================
  Files         144      147       +3     
  Lines       14964    15611     +647     
==========================================
+ Hits         7042     7909     +867     
+ Misses       6809     6453     -356     
- Partials     1113     1249     +136

Flag	Coverage Δ
e2etests	`47.23% <10.25%> (?)`
inttests	`44.01% <39.27%> (-0.42%)`	⬇️
unittests	`17.94% <0.27%> (-1.62%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
pkg/agent/config/config.go	`52.59% <ø> (+3.39%)`	⬆️
pkg/agent/containerd/containerd.go	`20.39% <ø> (ø)`
pkg/apis/k3s.cattle.io/v1/zz_generated_register.go	`69.23% <100.00%> (+5.59%)`	⬆️
pkg/cli/agent/agent.go	`26.22% <ø> (+26.22%)`	⬆️
pkg/cli/server/server.go	`62.33% <ø> (+2.20%)`	⬆️
pkg/cloudprovider/cloudprovider.go	`65.33% <ø> (ø)`
pkg/cloudprovider/servicelb.go	`62.21% <ø> (+0.20%)`	⬆️
pkg/cluster/address_controller.go	`84.21% <ø> (ø)`
pkg/cluster/bootstrap.go	`48.74% <100.00%> (+11.38%)`	⬆️
pkg/cluster/encrypt.go	`42.85% <100.00%> (-4.97%)`	⬇️
... and 30 more

... and 24 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

cwayne18 · 2023-09-28T16:00:10Z

cc @jakefhyde @Oats87 for any RM-related review

brandond · 2023-09-28T16:30:19Z

Moving to draft, this is still work in progress

Signed-off-by: Brad Davidson <brad.davidson@rancher.com>

Also adds a hack go script to print the embedded CRDs, for developer use. Signed-off-by: Brad Davidson <brad.davidson@rancher.com>

Signed-off-by: Brad Davidson <brad.davidson@rancher.com>

…estEntityTooLarge Signed-off-by: Brad Davidson <brad.davidson@rancher.com>

Consistently refer to object keys as such, simplify error handling. Signed-off-by: Brad Davidson <brad.davidson@rancher.com>

Attempt to use timestamp from creation or filename instead of file/object modification times Signed-off-by: Brad Davidson <brad.davidson@rancher.com>

Also, don't list ONLY s3 snapshots if S3 is enabled. Signed-off-by: Brad Davidson <brad.davidson@rancher.com>

Write the extra metadata both locally and to S3. These files are placed such that they will not be used by older versions of K3s that do not make use of them. Signed-off-by: Brad Davidson <brad.davidson@rancher.com>

Fixes snapshot list coming out in non-deterministic order Signed-off-by: Brad Davidson <brad.davidson@rancher.com>

Signed-off-by: Brad Davidson <brad.davidson@rancher.com>

Reconcile snapshot CRs instead of ConfigMap; manage ConfigMap downstream from CR list Signed-off-by: Brad Davidson <brad.davidson@rancher.com>

This required pulling the token hash stuff out of the cluster package, into util. Signed-off-by: Brad Davidson <brad.davidson@rancher.com>

Snapshot delete/prune tests were only working because the delete command would report success even when deleting a snapshot that didn't exist, and the test regex was finding the snapshot name multiple times in the list output and deleting it twice. Snapshot restore tests seem to have expected the deployment to be rolled out immediately, which is not a reasonable expectation. Signed-off-by: Brad Davidson <brad.davidson@rancher.com>

briandowns

Provisional approval pending CI.

brandond · 2023-10-12T22:01:44Z

S390x is having issues and we do not have an ETA for restoration; all other tests are green. merging

brandond requested a review from a team as a code owner July 28, 2023 00:24

brandond force-pushed the etcd-snapshot-cr branch 2 times, most recently from 3eef133 to 4df89b4 Compare July 28, 2023 05:50

manuelbuil reviewed Jul 28, 2023

View reviewed changes

docs/adrs/etcd-snapshot-cr.md Show resolved Hide resolved

manuelbuil approved these changes Jul 31, 2023

View reviewed changes

brandond force-pushed the etcd-snapshot-cr branch from 09995ae to 4df89b4 Compare August 1, 2023 00:58

brandond mentioned this pull request Aug 3, 2023

Etcd snapshots retention when node name changes #8099

Merged

brandond requested a review from Oats87 August 14, 2023 20:54

brandond force-pushed the etcd-snapshot-cr branch from ee69be6 to b89adc9 Compare September 28, 2023 00:42

brandond marked this pull request as draft September 28, 2023 16:29

brandond force-pushed the etcd-snapshot-cr branch 10 times, most recently from 886025d to 2eb5014 Compare October 6, 2023 00:43

brandond force-pushed the etcd-snapshot-cr branch 4 times, most recently from 84eddb1 to 3ceae0c Compare October 7, 2023 02:27

brandond force-pushed the etcd-snapshot-cr branch 3 times, most recently from 6bc6bc3 to 4854dbf Compare October 11, 2023 18:12

brandond added 14 commits October 12, 2023 19:17

Add ADR for etcd snapshot CRD migration

b6fbf3b

Signed-off-by: Brad Davidson <brad.davidson@rancher.com>

Minor updates as per design review discussion

96188cf

Signed-off-by: Brad Davidson <brad.davidson@rancher.com>

Add new CRD for etcd snapshots

4583892

Also adds a hack go script to print the embedded CRDs, for developer use. Signed-off-by: Brad Davidson <brad.davidson@rancher.com>

Move etcd snapshot code into separate file

cb08a45

Signed-off-by: Brad Davidson <brad.davidson@rancher.com>

Elide old snapshot data when apiserver rejects configmap with ErrRequ…

69b549d

…estEntityTooLarge Signed-off-by: Brad Davidson <brad.davidson@rancher.com>

Tidy s3 upload functions

df1cfc0

Consistently refer to object keys as such, simplify error handling. Signed-off-by: Brad Davidson <brad.davidson@rancher.com>

Consistently set snapshotFile timestamp

af03eaf

Attempt to use timestamp from creation or filename instead of file/object modification times Signed-off-by: Brad Davidson <brad.davidson@rancher.com>

Move s3 snapshot list functionality to s3.go

29cfd48

Also, don't list ONLY s3 snapshots if S3 is enabled. Signed-off-by: Brad Davidson <brad.davidson@rancher.com>

Store extra metadata and cluster ID for snapshots

c574a77

Write the extra metadata both locally and to S3. These files are placed such that they will not be used by older versions of K3s that do not make use of them. Signed-off-by: Brad Davidson <brad.davidson@rancher.com>

Sort snapshots by time and key in tabwriter output

7c346a4

Fixes snapshot list coming out in non-deterministic order Signed-off-by: Brad Davidson <brad.davidson@rancher.com>

Move snapshot delete into local/s3 functions

4d87b2c

Signed-off-by: Brad Davidson <brad.davidson@rancher.com>

Switch to managing ETCDSnapshotFile resources

58fb71e

Reconcile snapshot CRs instead of ConfigMap; manage ConfigMap downstream from CR list Signed-off-by: Brad Davidson <brad.davidson@rancher.com>

Add server token hash to CR and S3

87cc6a2

This required pulling the token hash stuff out of the cluster package, into util. Signed-off-by: Brad Davidson <brad.davidson@rancher.com>

brandond force-pushed the etcd-snapshot-cr branch from 4854dbf to 174ff79 Compare October 12, 2023 19:40

dereknola approved these changes Oct 12, 2023

View reviewed changes

briandowns approved these changes Oct 12, 2023

View reviewed changes

brandond merged commit 7c5b69c into k3s-io:master Oct 12, 2023
17 of 18 checks passed

This was referenced Oct 12, 2023

[release-1.27] Backports for 2023-10 release #8615

Merged

[release-1.26] Backports for 2023-10 release #8616

Merged

[release-1.25] Backports for 2023-10 release #8617

Merged

This was referenced Nov 1, 2023

Minor: k3s update from v1.28.2+k3s1 to v1.28.3+k3s1 cguertin14/k3s-ansible-ha#27

Merged

Minor: k3s update from v1.28.2+k3s1 to v1.28.3+k3s1 cguertin14/k3s-ansible-ha#28

Closed

brandond mentioned this pull request Mar 27, 2024

Fix etcd snapshot reconcile for agentless servers #9809

Merged

brandond mentioned this pull request Apr 29, 2024

leader-elected etcd controllers not consistently functional when leader election/lease mismatches occur #10046

Closed

brandond deleted the etcd-snapshot-cr branch June 6, 2024 21:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate to using custom resource to store etcd snapshot metadata #8064

Migrate to using custom resource to store etcd snapshot metadata #8064

brandond commented Jul 28, 2023 •

edited

Loading

manuelbuil left a comment

brandond commented Jul 28, 2023 •

edited

Loading

cwayne18 commented Aug 14, 2023

brandond commented Aug 14, 2023 •

edited

Loading

codecov bot commented Sep 28, 2023 •

edited

Loading

cwayne18 commented Sep 28, 2023

brandond commented Sep 28, 2023

briandowns left a comment

brandond commented Oct 12, 2023 •

edited

Loading

Migrate to using custom resource to store etcd snapshot metadata #8064

Migrate to using custom resource to store etcd snapshot metadata #8064

Conversation

brandond commented Jul 28, 2023 • edited Loading

Proposed Changes

Types of Changes

Verification

Testing

Linked Issues

User-Facing Change

Further Comments

manuelbuil left a comment

Choose a reason for hiding this comment

brandond commented Jul 28, 2023 • edited Loading

cwayne18 commented Aug 14, 2023

brandond commented Aug 14, 2023 • edited Loading

codecov bot commented Sep 28, 2023 • edited Loading

Codecov Report

cwayne18 commented Sep 28, 2023

brandond commented Sep 28, 2023

briandowns left a comment

Choose a reason for hiding this comment

brandond commented Oct 12, 2023 • edited Loading

brandond commented Jul 28, 2023 •

edited

Loading

brandond commented Jul 28, 2023 •

edited

Loading

brandond commented Aug 14, 2023 •

edited

Loading

codecov bot commented Sep 28, 2023 •

edited

Loading

brandond commented Oct 12, 2023 •

edited

Loading