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

chore: use k8s_apply=true by default #601

Merged
merged 1 commit into from
Oct 2, 2024
Merged

Conversation

mfocko
Copy link
Member

@mfocko mfocko commented Oct 1, 2024

• Summary

Initially I wanted to do ‹k8s_apply=false› for postgres, and key-value
databases (such as Redis, Redict, or Valkey), because deploying on prod
with ‹k8s_apply=true› caused redeployment of the postgres which caused
a small outage (~5 minutes).

Right now when I tried to redeploy stage multiple times in a row, none of
the deployed services got redeployed, hence I come to the conclusion that
there were some changes on the production deployment that were applied
back then.

• Context from #360

· What it actually does?

In the simple terms, it makes sure that the definition that is to be
deployed matches the one that is already deployed. The difference has
already manifested few times, e.g., when @majamassarini was adjusting
the `/dev/shm` for the postgres deployment (https://github.com/mfocko/deployment/commit/bedef2026c84ea00bb329799cc9bef81687fe88d), the change did
not get deployed.

· Why some tasks already use it (e.g., Redis/Redict, Flower secret) and
others not?

not sure

· Would it make sense to default to ‹apply=true›?

Yes, but at the same time, applying meaningless changes to critical
services, e.g., postgres or Redis/Redict/Valkey, can cause smaller
outages.

Fixes #360

@mfocko mfocko self-assigned this Oct 1, 2024
Copy link
Contributor

• Summary

Initially I wanted to do ‹k8s_apply=false› for postgres, and key-value
databases (such as Redis, Redict, or Valkey), because deploying on prod
with ‹k8s_apply=true› caused redeployment of the postgres which caused
a small outage (~5 minutes).

Right now when I tried to redeploy stage multiple times in a row, none of
the deployed services got redeployed, hence I come to the conclusion that
there were some changes on the production deployment that were applied
back then.

• Context from packit#360

  · What it actually does?

    In the simple terms, it makes sure that the definition that is to be
    deployed matches the one that is already deployed. The difference has
    already manifested few times, e.g., when @majamassarini was adjusting
    the `/dev/shm` for the postgres deployment (bedef20), the change did
    not get deployed.

  · Why some tasks already use it (e.g., Redis/Redict, Flower secret) and
    others not?

    not sure

  · Would it make sense to default to ‹apply=true›?

    Yes, but at the same time, applying meaningless changes to critical
    services, e.g., postgres or Redis/Redict/Valkey, can cause smaller
    outages.

Fixes packit#360

Signed-off-by: Matej Focko <[email protected]>
Copy link
Contributor

Copy link
Member

@lbarcziova lbarcziova left a comment

Choose a reason for hiding this comment

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

lgtm, we can revisit this if it starts causing bigger outages on redeployments

@mfocko
Copy link
Member Author

mfocko commented Oct 2, 2024

lgtm, we can revisit this if it starts causing bigger outages on redeployments

ofc we can, affects manual redeployments though, so not sure who remembers about it by the time we need to touch it manually :D even the TLS certs from Monday were without redeployment (I scaled stage, and Franta moved production, rebuilt images loaded new certs on automatic redeployment)

@mfocko mfocko merged commit ea14e87 into packit:main Oct 2, 2024
2 of 3 checks passed
@mfocko mfocko deleted the chore/k8s-apply branch October 2, 2024 07:09
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.

[Spike-ish] Use kubernetes.core.k8s apply=true by default?
2 participants