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

wait until deploy completes #26

Closed
jrynyt opened this issue Feb 24, 2017 · 17 comments
Closed

wait until deploy completes #26

jrynyt opened this issue Feb 24, 2017 · 17 comments

Comments

@jrynyt
Copy link

jrynyt commented Feb 24, 2017

Ideally kubectl apply would have something like a --wait option, so you know the actual rollout of Pods (etc) completed successfully, but it doesn't. So the success of a deploy here doesn't actually indicate the deploy succeeded, as the rollout happens asynchronously.

We could use kubectl rollout status after the kubectl apply, which blocks on deployment completion. It looks like you can even use -f to track all the resources in the config file.

@tonglil
Copy link
Member

tonglil commented Feb 24, 2017

This should be satisfied by #24 as you can run that command after apply, as well as run additional commands like kubectl get ....

@tonglil
Copy link
Member

tonglil commented Feb 24, 2017

It might also be a good idea to re-open #25 to use the -R option (and watch all resources in the folder).

@balser
Copy link
Contributor

balser commented Feb 24, 2017

I understand #24 is very flexible, but if there are a some common workflows/tasks related to the deploy, it might be better to be implemented once by the plugin vs implemented by each app owner.

@jrynyt
Copy link
Author

jrynyt commented Feb 27, 2017

I would consider this a bug, not an optional thing that devs might choose to do. The drone deploy should succeed/fail based on the success/failure of the k8s deploy.

Is #25 necessary for this to work? i.e. it would run in a separate container than the apply?

@tonglil
Copy link
Member

tonglil commented Feb 27, 2017

I can see this being a separate feature, will mark it as so.

It isn't a bug because this isn't a feature of kubectl apply, and is part of a larger feature request for multiple resource types to signal "ready".

The plugin will currently fail when the resource config spec fails, the same conditions as doing it via the command line.

@jrynyt
Copy link
Author

jrynyt commented Feb 27, 2017

I don't agree: the drone-gke plugin claims to "deploy container images to Kubernetes on Google Container Engine" -- not "run kubectl apply, which starts the deploy but doesn't wait for it to succeed or fail." kubectl apply isn't even mentioned in the doc, so a successful plugin execution but failed deploy looks like a bug.

See the conversation in #gke from 2/24.

"I know I’ve personally misconfigured my own app and only found out because I checked the logs."

@tonglil
Copy link
Member

tonglil commented Feb 27, 2017

The rollout command does not work for any other Kubernetes resources specified in the .kube.yml, which is why I am hesitant to add it as a parameter of the plugin since the template can contain any K8s resource.

The behavior that person described is the same as when using kubectl from the command line. Kubernetes is based on the model of convergence, and in certain scenarios things will never converge (out of resources, errors, resource limitations external to the cluster, etc).

I do not see the intersection between the plugin's claim and the behavior of Kubernetes. Maybe it should be "configure Kubernetes resources on Google Container Engine" instead to clarify?

@jrynyt
Copy link
Author

jrynyt commented Feb 28, 2017

I'll test how rollout behaves when other resources are in the kube files. I assumed it would know what to do (ignore) since it has -f and -R. I'm happy to do this work so I can learn more about drone and go.

The plugin does not say anywhere that it is a wrapper for kubectl, so I'm not sure how a user would intuit this behavior. My point is that if convergence fails, the deployment tool should not indicate a successful deploy.

@jrynyt
Copy link
Author

jrynyt commented Sep 14, 2017

@tonglil, @balser said you might be working on this?

@tonglil
Copy link
Member

tonglil commented Sep 14, 2017

Yes, it's a separate step/plugin.
I'll post the usage soon.

@jrynyt
Copy link
Author

jrynyt commented Sep 18, 2017

that doesn't sound like it will address the discussion we've been having above.

@jrynyt
Copy link
Author

jrynyt commented Oct 25, 2017

@tonglil, @balser and I talked about this. I'd like to implement optional parameters to this plugin to do kubectl rollout status and --prune. As you know, I think the former is a bug, and the latter will be more intuitive behavior. Both can be disabled by default.

Are you working on something that contradicts this?

@tonglil
Copy link
Member

tonglil commented Nov 14, 2017

Sorry @jrynyt didn't see this until now.

Check in with @chuhlomin, he did this recently (minus the --prune).

I am ok with this, as long as the user is aware that if they have multiple Deployment resources it will only watch one (unless the implementation infers/accepts multiple). Last I checked, kubectl rollout status -f breaks if there are more than one resources in the file (yes, I checked this and related issues kubernetes/kubernetes#20435).

With the pipeline design of Drone 0.5+, I was thinking that checking for rollout status can be done in a following step, and the user can choose to watch the rollout of multiple resources (eg n Deployments, m DaemonSets, and i StatefulSets) like:

wait:
  image: tonglil/auth-gke
  environment:
    - PROJECT=xyz
    - ZONE=abc 
    - CLUSTER=cluster
  secrets:
    - source: GCP_JSON_KEY
      target: TOKEN
  commands:
    - auth-gke
    - kubectl rollout status deploy/xxx
    - kubectl rollout status daemonset/yyy
    - kubectl rollout status statefulset/zzz

@chuhlomin
Copy link
Contributor

Hi, that's right, I've implemented optional rollout check with option timeout for this: develop...chuhlomin:develop
Example usage:

    rollout_check: [bodega-server, bodega-server-envoy]
    rollout_timeout: 300

Example output:

$ timeout -t 120 kubectl rollout status deploy gateway-server --namespace pubp-dev
Waiting for rollout to finish: 2 old replicas are pending termination...
Waiting for rollout to finish: 2 old replicas are pending termination...
Waiting for rollout to finish: 2 old replicas are pending termination...
Waiting for rollout to finish: 2 old replicas are pending termination...
deployment "gateway-server" successfully rolled out

I didn't think about deamonset or statefulset, but it would be easy to add this functionality.

@jrynyt
Copy link
Author

jrynyt commented Dec 11, 2017

@chuhlomin that's great -- can you open a PR and add team/deliveryengineering as reviewers?

@chuhlomin
Copy link
Contributor

Done, but I cannot set reviewers.

@tonglil
Copy link
Member

tonglil commented Jan 25, 2018

Closing.

Please create a new issue for additional features (such as --prune).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants