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

Revisit LoadBalancer deletion logic #195

Open
jvalkeal opened this issue Mar 9, 2018 · 1 comment
Open

Revisit LoadBalancer deletion logic #195

jvalkeal opened this issue Mar 9, 2018 · 1 comment
Labels
type/technical-debt Techical Dept

Comments

@jvalkeal
Copy link
Contributor

jvalkeal commented Mar 9, 2018

Current logic of deleting LoadBalancer service during undeploy have few issues.

  • We should not silently eat InterruptedException as it may be a signal that this task should be aborted. This happens if this is run within cancellable task in TaskExecutor.
  • We wait LoadBalancer to go away which will never happen as we never ask to delete it.
  • For example with minikube, there's no real LoadBalancer while service itself is created. When service response have empty LoadBalancer, svc.getStatus().getLoadBalancer().getIngress().isEmpty() will always return true, thus we always wait.

if (svc != null && "LoadBalancer".equals(svc.getSpec().getType())) {
int tries = 0;
int maxWait = properties.getMinutesToWaitForLoadBalancer() * 6; // we check 6 times per minute
while (tries++ < maxWait) {
if (svc.getStatus() != null && svc.getStatus().getLoadBalancer() != null
&& svc.getStatus().getLoadBalancer().getIngress() != null && svc.getStatus()
.getLoadBalancer().getIngress().isEmpty()) {
if (tries % 6 == 0) {
logger.warn("Waiting for LoadBalancer to complete before deleting it ...");
}
logger.debug(String.format("Waiting for LoadBalancer, try %d", tries));
try {
Thread.sleep(10000L);
}
catch (InterruptedException e) {
}
svc = client.services().withName(appIdToDelete).get();
}
else {
break;
}
}
logger.debug(String.format("LoadBalancer Ingress: %s",
svc.getStatus().getLoadBalancer().getIngress().toString()));
}

@sabbyanandan
Copy link
Contributor

We would have to look into to see why it was decided to confirm that the objects are actually deleted from the K8s cluster. This may not be necessary after all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/technical-debt Techical Dept
Projects
None yet
Development

No branches or pull requests

2 participants