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

Investigate deployers deletion when cancelling deployments #8439

Closed
0xmichalis opened this issue Apr 9, 2016 · 9 comments
Closed

Investigate deployers deletion when cancelling deployments #8439

0xmichalis opened this issue Apr 9, 2016 · 9 comments

Comments

@0xmichalis
Copy link
Contributor

Currently when we cancel a running deployment, we set ActiveDeadlineSeconds to its deployers so that they will fail and subsequently transition the deployment phase to Failed. We should investigate deleting those deployers.

cc: @smarterclayton @ironcladlou @pweil-

@0xmichalis
Copy link
Contributor Author

A pretty simple fix would be to retain the current behavior of cancelling by setting ADS when New or Running, and cleanup when Complete (already happens) or Failed + Cancelled.

@soltysh
Copy link
Contributor

soltysh commented Apr 11, 2016

Isn't migration to upstream deployment going to solve the problem of the deployer? IIRC they're not using any. On the other hand, deployer was created to allow creating users one by themselves, isn't this going to be backward compatibility issue?

@0xmichalis
Copy link
Contributor Author

Isn't migration to upstream deployment going to solve the problem of the deployer? IIRC they're not using any.

I think deployers will be always needed for running custom strategies. Also, until we get full feature parity, we will defer to our controller for feaures supported only by us.

On the other hand, deployer was created to allow creating users one by themselves, isn't this going to be backward compatibility issue?

This issue is about cleaning up deployers on cancelled deployments. I don't see any backwards compatibility implications but maybe I am missing something. Why do you think so?

@soltysh
Copy link
Contributor

soltysh commented Apr 11, 2016

This issue is about cleaning up deployers on cancelled deployments. I don't see any backwards compatibility implications but maybe I am missing something. Why do you think so?

I understood your 'total removal' as entirely removing them ;)

@0xmichalis
Copy link
Contributor Author

reworded

@ironcladlou
Copy link
Contributor

I think it's arguable that cancelled deployers should be deleted, but I guess it's possible a user notices something's wrong and hits the cancel button so they can go inspect the pods immediately.

This is UX guesswork, so I'm not sure there's a right answer. Where's the use case coming from?

@0xmichalis
Copy link
Contributor Author

By "the pods" I assume you mean the deployers since the new application pods will be scaled down. Won't/Shouldn't the deployment fail either way if something wrong is noticed by the process?

@smarterclayton
Copy link
Contributor

Does a user need the deployer pod if the deployment is cancelled? We
used to require the pod to have logs, but in theory we have a solution
for that. We have a card in plat-man to have logs for deployments / builds
/ pods that have been deleted be fetched from the logging service. Would
that be sufficient that cancel deleting the deployer pod would be
sufficient?

On Mon, Apr 11, 2016 at 9:41 AM, Michail Kargakis [email protected]
wrote:

By "the pods" I assume you mean the deployers since the new application
pods will be scaled down. Won't the deployment fail either way if something
wrong is noticed by the process?


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#8439 (comment)

@0xmichalis
Copy link
Contributor Author

Yeah, I don't think the deployer is any useful in case of a cancellation. I would say we could go ahead and remove deployers before we bring the logging service into play but they (deployer pods) would definitely be obsolete post-deployment in case of such an addition (fetch logs from deleted pods)

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

7 participants