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

How to do a rolling restart of a deployment? #630

Closed
vifino opened this issue Sep 9, 2021 · 12 comments · Fixed by #635
Closed

How to do a rolling restart of a deployment? #630

vifino opened this issue Sep 9, 2021 · 12 comments · Fixed by #635
Labels
good first issue Good for newcomers help wanted Not immediately prioritised, please help! utils extra utility modules

Comments

@vifino
Copy link
Contributor

vifino commented Sep 9, 2021

Hello.

I'm developing a project which needs to occasionally restart deployments.
There does not seem to be much special handling for deployments, only for pods.

Could you be so kind as to show me how to something akin to kubectl rollout restart?
Thank you so much.

@clux
Copy link
Member

clux commented Sep 9, 2021

kubectl rollout restart just sets an annotation on the deployment spec to trigger the restart AFAIKR (you can inspect it with kubectl -v=9 rollout restart).

We don't have a lot of special object handling herein, although i believe the trick that kubectl rollout restart is doing should work for any workload. You could do something similar to that for the time being.

Going to mark this as a potential utils inclusion.

@clux clux added the utils extra utility modules label Sep 9, 2021
@clux
Copy link
Member

clux commented Sep 9, 2021

For clarity: kubectl effectively patches a kubectl.kubernetes.io/restartedAt on workload.spec.template.metadata.annotations

spec:
  template:
    metadata:
      annotations:
        kubectl.kubernetes.io/restartedAt: "2021-09-09T11:05:22+01:00"

This can be done with the Api<Deployment> instance (say) via something like (untested):

    let patch = serde_json::json!({
      "apiVersion": "apps/v1",
      "kind": "Deployment",
      "spec": {
        "template": {
          "metadata": {
            "annotations": {
              "kube.kubernetes.io/restartedAt": Utc::now().to_rfc3339()
            }
          }
        }
      }
    });

    let res = api.patch(mydeploy, &ssapply, &Patch::Apply(patch)).await?;

But I think we should have some helper behaviour like this in a utils module. E.g. a Restart trait that we implement for core workloads, which adds method that does something like this.

@vifino
Copy link
Contributor Author

vifino commented Sep 14, 2021

Hello @clux, thank you for your further comment.
It was very informative.

I've made your snippet compile in my code (but didn't get a chance to test it yet), but I'd definitly enjoy a generic Restart trait, after all, this might be something one would do more often.
Once I'm sure that snippet works, I'd gladly spend some (personal) time to help this crate's efforts. :)

Thank you so much!

@vifino
Copy link
Contributor Author

vifino commented Sep 14, 2021

Hello again.
I finally tried it out and figured it out.

After a bit of trouble I had with not specifying the field_manager, I got this snippet to work just fine for me:

    /// Restart a Kubernetes Deployment by name.
    async fn restart_deployment(&self, name: &str) -> Result<()> {
        let deployments: Api<Deployment> = Api::namespaced(self.client.clone(), "<SNIP>");

        let patch = serde_json::json!({
          "apiVersion": "apps/v1",
          "kind": "Deployment",
          "spec": {
            "template": {
              "metadata": {
                "annotations": {
                  "kube.kubernetes.io/restartedAt": Utc::now().to_rfc3339()
                }
              }
            }
          }
        });
        let mut patchparams = PatchParams::default();
        patchparams.field_manager = Some(String::from("merge"));

        deployments
            .patch(name, &patchparams, &Patch::Apply(patch))
            .await?;
        Ok(())
    }

Thank you once again and I'd be very glad to assist in adding a Restart trait here.
Let me know what I can do.

PS: Sorry for the double-post. Too little caffeine.

@clux
Copy link
Member

clux commented Sep 14, 2021

Awesome, that's great to hear. You can use let patchparams = PatchParams::apply("kube") for a more explicit shorthand.

I think this would be a great addition that's probably not too bad to contribute first time.
If you are able to help then that's great! I'll write out my general thoughts on how it would be best served:

Sketch

  • new file kube-core/src/util.rs with:
    • pub trait Restart {} marker trait
    • implement the marker trait for k8s_openapi::...{Pod, Deployment, DaemonSet, StatefulSet, ReplicaSet} (don't think this makes sense for cronjobs / jobs?)
    • an impl Request { fn restart(&name) -> Result<Vec<u8>> {...} } to create the http request (ala the sibling subresource.rs file)
  • new file kube/src/api/util.rs with:
    • the generic implementation on Api:
impl<K> Api<K>
where K: Restart + More? {
    pub fn restart(name: &str) -> Result<K> { /*call self.requests.restart(name) and use it with the client (like most api calls)*/ }
}

@clux clux added good first issue Good for newcomers help wanted Not immediately prioritised, please help! labels Sep 14, 2021
@vifino
Copy link
Contributor Author

vifino commented Sep 14, 2021

That sounds like a good plan.
I agree that on Jobs, it does not make that much sense. You could just set a decent deadline.

I'd implement the restart on the Request by just calling self.patch.
However, implementing it completely generically won't work given that you need to set the kind in the patch.
Passing it further up the implementation on Api also does not solve the problem.

So my questions before I start:

  • Is there a way I can get the "kubernetes name" of K? (Maybe K's type name?)
  • The returned K would be with the patch applied, correct?

@nightkr
Copy link
Member

nightkr commented Sep 14, 2021

You probably don't want to use server-side apply for this, since that will prevent anyone else from updating that annotation for any other reason (such as kubectl itself). Consider using a merge patch instead?

Also, field_manager, if set, must uniquely identify that application, purpose, and call site (so rook-ceph-mon-restarter would be fine, while merge will end in tears sooner or later).

@nightkr
Copy link
Member

nightkr commented Sep 14, 2021

Is there a way I can get the "kubernetes name" of K? (Maybe K's type name?)

You should be able to use obj.kind(api.dyntype).

@clux
Copy link
Member

clux commented Sep 14, 2021

You can get the kind by requiring K to be a Resource, and all of DaemonSet/Deployment/Statefulset can re-use the same patch apart from the kind (which you can grab in).

Pods won't work with the same patch, but also restarting pods would be different. You'd probably delete the pod. Even kubectl doesn't support it:

error: pods "foo-controller-69575855bf-h9d4k" restarting is not supported

@clux
Copy link
Member

clux commented Sep 14, 2021

@teozkr : would the field-manager thing be a problem if we are patching with the annotation "kube.kubernetes.io/restartedAt" ? It would only cause problems if other controllers tried to mess with that annotation right?

EDIT: In particular, this is what happens when you do it with kubectl, it adds a field-manager called kubctl-rollout:

    - apiVersion: apps/v1
      fieldsType: FieldsV1
      fieldsV1:
        f:spec:
          f:template:
            f:metadata:
              f:annotations:
                f:kubectl.kubernetes.io/restartedAt: {}
      manager: kubectl-rollout
      operation: Update
      time: "2021-09-14T15:23:12Z"

EDIT: kubectl rollout source

@vifino
Copy link
Contributor Author

vifino commented Sep 14, 2021

I've changed it to be a merge patch and since field_manager is optional in this case, I'll just not set it.
However, I looked at the resulting object and it didn't add any managedFields field in the metadata. Don't think it matters, but I'll keep the change.

I'll require K to be Resource and call .kind() on it. After all, that seems the least hacky.

@vifino
Copy link
Contributor Author

vifino commented Sep 14, 2021

Actually, since the patch method already selects the right object, adding apiVersion and kind is not necessary.
After all, it does not need to change that. Even better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Not immediately prioritised, please help! utils extra utility modules
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants