-
Notifications
You must be signed in to change notification settings - Fork 0
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
delete application #84
Conversation
* no safeguard for deleting deployed application images * removes revision deletion endpoint and interfaces
* adds k8s cluster config to start up * adds test for app delete * adds local repo deletion
main.go
Outdated
@@ -53,6 +53,11 @@ func main() { | |||
|
|||
kiln.LogInfo.Printf("ImageCreator set up.") | |||
|
|||
clusterConfig, err := kiln.NewClusterConfig() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should support out of cluster kubernetes client like we do in enrober/dispatcher as well. See: https://github.com/30x/enrober/blob/staging/pkg/server/server.go#L83-L113
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made a function to get a local config. I'll follow the config for this that y'all are doing.
pkg/kiln/deploy_check.go
Outdated
} | ||
|
||
// uses the current context in kubeconfig | ||
config, err := clientcmd.BuildConfigFromFlags("", path.Join(usr.HomeDir, ".kube/config")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Kubernetes clientcmd provides a way of doing this automatically instead of generating when the kube config is by hand. See: https://godoc.org/k8s.io/client-go/tools/clientcmd#BuildConfigFromFlags
It's the same code we use enrober and dispatcher I would vote we use the same logic in all three projects. https://github.com/30x/dispatcher/blob/master/kubernetes/client.go#L54-L62 and https://github.com/30x/enrober/blob/staging/pkg/server/server.go#L97-L111.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree. I do think that if there are more than a few lines of code to use this in each project that we bubble it up into a shared package. But we can do that later.
pkg/server/server.go
Outdated
|
||
if active { | ||
w.WriteHeader(http.StatusConflict) | ||
w.Header().Set("Content-Type", "text/plain charset=utf-8") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All of our API errors should be JSON. I think enrober
is already returning an error structure so you might want to look at that for the existing pattern.
Fixes #83
Fixes
kiln
's part in #3