Skip to content
This repository has been archived by the owner on Jan 8, 2024. It is now read-only.

Fix a few minor kubernetes papercuts #2399

Merged
merged 3 commits into from
Oct 1, 2021
Merged

Fix a few minor kubernetes papercuts #2399

merged 3 commits into from
Oct 1, 2021

Conversation

evanphx
Copy link
Contributor

@evanphx evanphx commented Sep 30, 2021

  1. This scales the wait time back so that bad deploys fail much faster.
    Previously we had it hardcoded to 10 minutes, now it's dynamic.
  2. When a deployment fails, print out non Info events from the pods to
    help the user diagonise what happened.
  3. Provide some small grace period on pod cleanup to allow for the
    potential of debugging the failed pods directly.
  4. Handle not being able to search for metric pods
  5. Handle destroying an old deployment that didn't have autoscaling
    defined yet.
  6. Setup the kubernetes service with the workspace name if the workspace
    is not the default workspace

1. This scales the wait time back so that bad deploys fail much faster.
   Previously we had it hardcoded to 10 minutes, now it's dynamic.
2. When a deployment fails, print out non Info events from the pods to
   help the user diagonise what happened.
3. Provide some small grace period on pod cleanup to allow for the
   potential of debugging the failed pods directly.
4. Handle not being able to search for metric pods
5. Handle destroying an old deployment that didn't have autoscaling
   defined yet.
6. Setup the kubernetes service with the workspace name if the workspace
   is not the default workspace
@evanphx evanphx requested a review from a team September 30, 2021 23:18
@evanphx evanphx added pr/no-changelog No automatic changelog entry required for this pull request plugin/k8s plugin and removed plugin plugin/k8s pr/no-changelog No automatic changelog entry required for this pull request labels Sep 30, 2021
ui terminal.UI,
target *Deployment,
dcr *component.DeclaredResourcesResp,
) (*Release, error) {
var result Release
result.ServiceName = src.App

if job.Workspace == "default" {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should throw this in the labels too for the label selector.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure what ya mean....

Copy link
Member

@briancain briancain left a comment

Choose a reason for hiding this comment

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

I've got a few minor requests and comments, but this looks great! ✨ 💯

.changelog/2399.txt Outdated Show resolved Hide resolved
builtin/k8s/platform.go Outdated Show resolved Hide resolved
dep, err := dc.Get(ctx, result.Name, metav1.GetOptions{})
if err != nil {
return false, err
}

if time.Since(lastStatus) > 10*time.Second {
step.Update(fmt.Sprintf(
"Waiting on deployment to become available: %d/%d/%d",
"Waiting on deployment to become available: requested=%d running=%d ready=%d",
Copy link
Member

Choose a reason for hiding this comment

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

I noticed this change in your demo and I like it! 👍🏻 ✨

Copy link
Contributor Author

Choose a reason for hiding this comment

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

bitmoji

builtin/k8s/platform.go Show resolved Hide resolved
if ev.Type == "Normal" {
continue
}
ui.Output(" %s: %s (%s)", ev.Type, ev.Message, ev.Reason)
Copy link
Member

Choose a reason for hiding this comment

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

I also noticed this change in your demo, excited for this! 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

bitmoji

@evanphx evanphx merged commit 0bdca78 into main Oct 1, 2021
@evanphx evanphx deleted the f-k8-papercuts branch October 1, 2021 20:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants