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
PAAS-286 mark pod as unscheduleable when it has abnormal events #1312
Conversation
def show_failure_cause(release) | ||
bad_pods(release).each do |pod, client, deploy_group| | ||
def show_failure_cause(release, release_docs) | ||
pod_statuses(release, release_docs).reject(&:live).select(&:pod).each do |status| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
previously this was duplicating the logic from release_statuses
unless successful | ||
show_failure_cause(release) | ||
rollback(deploys) | ||
show_failure_cause(release, release_docs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
previously this showed all failed deploys ... which was never a real issue since if something failed the previous deploy would have failed ... , but this should be more correct
b925782
to
ffeab78
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally fine, just a few small suggestions.
@@ -48,6 +50,27 @@ def containers | |||
@pod.spec.containers | |||
end | |||
|
|||
def logs(container) | |||
client.get_pod_log(name, namespace, previous: restarted?, container: container) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would crash if you tried calling this without first calling client=
, right? If you have methods that expect there to be a valid client object, you should probably add it as a parameter to the constructor.
def stopped? | ||
if @stopped | ||
@output.puts "STOPPED" | ||
true | ||
end | ||
end | ||
|
||
# TODO: cleanup ... a bit ugly with all these arrays |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you have 4-tuples of values, we should probably just create an object to encapsulate the data.
ffeab78
to
0b4cfe5
Compare
@jonmoter @irwaters