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.

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

[coord] New secret guarantees #12334

Merged
merged 10 commits into from
May 11, 2022
Merged

[coord] New secret guarantees #12334

merged 10 commits into from
May 11, 2022

Conversation

mkysel
Copy link
Contributor

@mkysel mkysel commented May 9, 2022

We have decided to move the creation of the secret into the environment controller in Kubernetes. When using the Kubernetes environment, all scripts have to create a new user-managed-secrets secret, if they desire to use the Kubernetes controller.

The KubernetesSecretsController now waits until the secret becomes visible on its local filesystem. When altering secrets, we do not guarantee that the new secret content has been propagated.

Motivation

This PR adds a known-desirable feature.

Testing

  • This PR has adequate test coverage / QA involvement has been duly considered.

Tested manually. Here are the current execution times on a default Kubernetes installation:

root@psql:/# time psql postgres://materialize:materialize@materialized.default.svc.cluster.local:6875/materialize -c "create secret s as 'bar'";
CREATE SECRET

real	0m0.252s
user	0m0.027s
sys	0m0.009s
root@psql:/# time psql postgres://materialize:materialize@materialized.default.svc.cluster.local:6875/materialize -c "alter secret s as 'bar'";
ALTER SECRET

real	0m0.071s
user	0m0.026s
sys	0m0.010s
root@psql:/# time psql postgres://materialize:materialize@materialized.default.svc.cluster.local:6875/materialize -c "drop secret s";
DROP SECRET

real	0m0.083s
user	0m0.026s
sys	0m0.010s

@mkysel mkysel marked this pull request as ready for review May 9, 2022 15:17
Copy link
Contributor

@philip-stoev philip-stoev left a comment

Choose a reason for hiding this comment

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

As discussed with @benesch , secrets will be the first thing to test once we have a kubernetes-enabled testing framework.

@mkysel
Copy link
Contributor Author

mkysel commented May 9, 2022

thank you @philip-stoev. Looking forward to it.

bin/cluster-reset Outdated Show resolved Hide resolved
src/secrets-kubernetes/src/lib.rs Outdated Show resolved Hide resolved
src/secrets-kubernetes/src/lib.rs Outdated Show resolved Hide resolved
src/secrets-kubernetes/src/lib.rs Outdated Show resolved Hide resolved
src/secrets-kubernetes/src/lib.rs Outdated Show resolved Hide resolved
@mkysel
Copy link
Contributor Author

mkysel commented May 9, 2022

adding @alex-hunt-materialize for FYI

Copy link
Member

@benesch benesch left a comment

Choose a reason for hiding this comment

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

This looks about right to me, but would be good to get Alex's eyes on the secret pod update too!

src/materialized/src/lib.rs Outdated Show resolved Hide resolved
src/secrets-kubernetes/src/lib.rs Outdated Show resolved Hide resolved
src/secrets-kubernetes/src/lib.rs Outdated Show resolved Hide resolved
src/secrets-kubernetes/src/lib.rs Outdated Show resolved Hide resolved
src/materialized/src/bin/materialized/main.rs Outdated Show resolved Hide resolved
@mkysel mkysel requested review from benesch and guswynn May 11, 2022 14:42
Copy link
Member

@benesch benesch left a comment

Choose a reason for hiding this comment

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

Looks great!

@mkysel mkysel merged commit ebd6875 into MaterializeInc:main May 11, 2022
@mkysel mkysel deleted the coordd-secret-guarantees branch May 11, 2022 15:27
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.

5 participants