Skip to content

cli: down works on gyaml#910

Merged
jazzdan merged 4 commits into
masterfrom
maiamcc/down-works-on-gyaml
Jan 3, 2019
Merged

cli: down works on gyaml#910
jazzdan merged 4 commits into
masterfrom
maiamcc/down-works-on-gyaml

Conversation

@jazzdan

@jazzdan jazzdan commented Jan 3, 2019

Copy link
Copy Markdown
Contributor

From Kubernetes slack:

Quick question: should tilt down delete all resources created from k8s_yaml? I’m not seeing an easy way to clean up configmaps etc created by tilt up.

I’m seeing this with any resource that doesn’t reference an image that tilt knows about… so if my Tiltfile just contains

k8s_yaml("pod.yaml")

tilt up creates the pod, but it remains after tilt down

We simply weren't also deleting the global YAML resources that we record in the Tiltfile when down is called.

While I was here I made a change to the k8s client to print out which resources we're deleting, similarly to how we do it in Upset. Looks like this:

[~/go/src/github.com/windmilleng/servantes]> tilt down
Deleting via kubectl: Deployment/dan-fe
Deleting via kubectl: Service/dan-fe
Deleting via kubectl: Deployment/dan-vigoda
Deleting via kubectl: Deployment/dan-snack
Deleting via kubectl: Deployment/dan-doggos
Deleting via kubectl: Deployment/dan-fortune
Deleting via kubectl: Deployment/dan-hypothesizer
Deleting via kubectl: Service/dan-hypothesizer
Deleting via kubectl: Deployment/dan-spoonerisms
Deleting via kubectl: Deployment/dan-emoji
Deleting via kubectl: Deployment/dan-words
Deleting via kubectl: Deployment/dan-secrets
Deleting via kubectl: Service/dan-secrets
Deleting via kubectl: Role/pod-reader
Deleting via kubectl: RoleBinding/read-pods
Deleting via kubectl: Secret/dan-servantes-stuff

@jazzdan
jazzdan requested review from maiamcc and nicks January 3, 2019 17:51
@jazzdan

jazzdan commented Jan 3, 2019

Copy link
Copy Markdown
Contributor Author

I didn't write a test because it seemed hard to test and isn't testing a whole lot of logic. Open to ideas though!

@maiamcc maiamcc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

Comment thread internal/k8s/client.go
pf PortForwarder) K8sClient {

// TODO(nick): I'm not happy about the way that pkg/browser uses global writers.
writer := logger.Get(ctx).Writer(logger.DebugLvl)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why did you remove this? this is pretty important

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah I realized that and added it back.

Comment thread internal/k8s/client.go Outdated
for _, e := range entities {
b.WriteString(fmt.Sprintf("Deleting via kubectl: %s/%s\n", e.Kind.Kind, e.Name()))
}
l.Infof(b.String())

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This will crash if the string contains any % signs.

You should either use Infof inside the loop (rather than have a separate Printf) or use something like Infof("%s", string)

@jazzdan
jazzdan merged commit a4f0041 into master Jan 3, 2019
@maiamcc
maiamcc deleted the maiamcc/down-works-on-gyaml branch February 5, 2019 17:14
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.

3 participants