[backend] kubernetes: fix secret size limitation#29678
Conversation
9930bf6 to
cfa8df9
Compare
|
This is basically a similar fix as #28838, but for the kubernetes state file. |
cfa8df9 to
fdd5ecd
Compare
|
Thanks for this submission! I will raise this with the internal maintainers of the kubernetes backend. Although I cannot commit to having this PR reviewed at this time, we acknowledge your contribution and appreciate it! Thanks again for the submission! |
fdd5ecd to
948bb18
Compare
948bb18 to
807c5d1
Compare
|
Just making noise to this PR, is this a ready to go? |
alexsomesan
left a comment
There was a problem hiding this comment.
It was a bit difficult to comprehend certain parts of this change so I left some spot questions that might hopefully clarify it.
Also, a more general question: is there potential for conflict if more than one terraform workspace is stored in the same K8s namespace?
| }, | ||
| }, | ||
| } | ||
| secret, err = c.kubernetesSecretClient.Create(ctx, secret, metav1.CreateOptions{}) |
There was a problem hiding this comment.
AFAIK the Create call translates into a POST HTTP method. How does this behave when the Secret for the chunk already exists?
There was a problem hiding this comment.
The logic above this checks if the secret already exists or not and only calls create if it does not already exist. We could however reformulate this as a single patch call instead. This is just logic that was already present for the single secret backend, it has just been shuffled around in this PR.
bad6c66 to
7f1188e
Compare
The purpose of the |
|
Was there a specific reason why this was not merged and released after @alexsomesan approved it in february? This feature would still be very much appreciated. |
|
Looks like it got missed. I will add it to the triage queue. Thanks! |
By now kubernetes backend could hold up to defaultETCDSize gzipped data (which is 1-1.5Mb). This doesn't scale for larger states. This commit implements spliting data across multiple secrets bound by the same Secret labels. This practically removes etcd value size limitation and allows backend to scale across multiple secrets. This also takes care of cases when state needs to be shrinked. In such case code will cleanup unneeded secrets Signed-off-by: Dinar Valeev <dinar.valeev@absa.africa>
dd84fa3 to
119a8c0
Compare
|
Thanks for your patience, everyone! |
|
Reminder for the merging maintainer: if this is a user-visible change, please update the changelog on the appropriate release branch. |
|
When the data are gzip encoded, what I'd expect to find is that each Secret holds a valid gzip stream. Concatenating these also produces a gzip stream that you can uncompress and deserialize. Is that how it works? |
|
💭 I wonder whether and how we provide atomic updates when there are multiple Secrets |
|
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
By now kubernetes backend could hold up to defaultETCDSize gzipped data
(which is 1-1.5Mb). This doesn't scale for larger states.
This commit implements spliting data across multiple secrets bound by
the same Secret labels. This practically removes etcd value size
limitation and allows backend to scale across multiple secrets.
This also takes care of cases when state needs to be shrinked. In such
case code will cleanup unneeded secrets.
Signed-off-by: Dinar Valeev dinar.valeev@absa.africa