Skip to content

Conversation

@kwmonroe
Copy link
Contributor

@kwmonroe kwmonroe commented Apr 7, 2022

Lots of things don't work when control plane subordinates depart. kubectl get $stuff is especially troublesome as it takes forever to timeout. When we know our subs are going away, don't run things that call kubectl get $stuff like get_dns_ip and configure_cdk_addons.

Fixes https://bugs.launchpad.net/charm-kubernetes-master/+bug/1968237

@kwmonroe kwmonroe changed the title guard pesky functions on cni.deparated better handling of subordinate departed states Apr 8, 2022
@kwmonroe
Copy link
Contributor Author

kwmonroe commented Apr 8, 2022

Looks good:

$ time juju destroy-model diehard -y
Destroying model
Waiting for model to be removed, 10 machine(s), 7 application(s)................
......
Waiting for model to be removed, 9 machine(s), 7 application(s)....
Waiting for model to be removed, 8 machine(s), 7 application(s)......
Waiting for model to be removed, 7 machine(s), 7 application(s)...
Waiting for model to be removed, 6 machine(s), 7 application(s)........
Waiting for model to be removed, 5 machine(s), 7 application(s).................
..
Waiting for model to be removed, 4 machine(s), 7 application(s)...
Waiting for model to be removed, 3 machine(s), 7 application(s)...
Waiting for model to be removed, 2 machine(s), 7 application(s).................
...............................
Waiting for model to be removed, 2 machine(s), 6 application(s)...............
Waiting for model to be removed, 2 machine(s), 5 application(s).............
Waiting for model to be removed, 2 machine(s), 4 application(s).................
.
Waiting for model to be removed, 2 machine(s), 3 application(s).............
Waiting for model to be removed, 2 machine(s), 2 application(s).........
Waiting for model to be removed, 1 machine(s), 2 application(s).................
.....
Waiting for model to be removed, 1 machine(s), 1 application(s).................
.......
Waiting for model to be removed, 1 machine(s)....
Model destroyed.

real	6m57.537s
user	0m0.240s
sys	0m0.295s

7 minutes is considerably better than the ♾️ minutes it was taking before.

@kwmonroe kwmonroe marked this pull request as ready for review April 8, 2022 04:22
@kwmonroe kwmonroe marked this pull request as draft April 8, 2022 14:06
@kwmonroe
Copy link
Contributor Author

kwmonroe commented Apr 8, 2022

/hold to flesh this out in charmed-kubernetes/layer-kubernetes-common#28

Copy link
Member

@addyess addyess left a comment

Choose a reason for hiding this comment

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

Yep, this look pretty nice.

Comment on lines +1401 to 1404
if dns_ip:
return True, dns_ip, 53, dns_domain
else:
return False, None, None, None
Copy link
Member

Choose a reason for hiding this comment

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

Random nit: this method could REALLY use some return typing

def get_dns_info() -> Tuple[bool, Optional[str], Optional[int], Optional[str]]:

Suggested change
if dns_ip:
return True, dns_ip, 53, dns_domain
else:
return False, None, None, None
if dns_ip:
return True, dns_ip, 53, dns_domain
return False, None, None, None

@addyess
Copy link
Member

addyess commented Apr 5, 2023

@kwmonroe i'm still good with this one

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants