Skip to content

Conversation

@takirala
Copy link
Contributor

@takirala takirala commented Mar 18, 2019

DCOS_OSS-5013

This is for 1.12 and above. We stopped supporting zip universes starting 1.8 and this PR removes the deadcode. This is needed to removed unnecessary metrics (See #771 ). This also bring couple other changes:

  • Always log exceptions in full. Lack of this makes debugging issues like DCOS-46647 difficult.
  • Do not retry uninstall when the auth token is expired.

@takirala takirala self-assigned this Mar 18, 2019
@takirala takirala force-pushed the tga/remove-zip-universe branch from fe6e15b to 40079dd Compare April 3, 2019 20:20
}
}
}.rescue {
case ce: CosmosException if ce.error.isInstanceOf[Unauthorized] =>
Copy link
Contributor Author

@takirala takirala Apr 4, 2019

Choose a reason for hiding this comment

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

Context : Currently cosmos does not have any sort of authz. It relies on AR to forward an auth token which it uses when it is talking to marathon (or any other service). If the uninstall does not complete before the auth token expires, cosmos will keep polling marathon with the expired token. We should fail fast when this happens.

Ideally, cosmos can implement authz and refresh the token on expiry. But until then, we can stop retrying when we know retry will fail. See this slack convo https://mesosphere.slack.com/archives/CAA1Z7NKT/p1553816267233400 on how it affected MWT. cc @timcharper

Copy link
Contributor

@alenkacz alenkacz left a comment

Choose a reason for hiding this comment

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

Less code is always better :D! 👍

@takirala takirala merged commit 04b70bf into master Apr 8, 2019
@takirala takirala deleted the tga/remove-zip-universe branch April 8, 2019 17:25
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.

4 participants