Deletions (third attempt) #123
Conversation
| @@ -10,17 +11,17 @@ import ( | |||
| ) | |||
|
|
|||
| // Apply applies the given yaml to the cluster | |||
| func (k Kubectl) Apply(data manifest.List, opts ApplyOpts) error { | |||
| func (k Kubectl) Apply(labels []string, data manifest.List, opts ApplyOpts) error { | |||
There was a problem hiding this comment.
The labels are not required if not running with prune, right? If so, then please move them into ApplyOpts
There was a problem hiding this comment.
Not quite. The labels are needed in Apply in order to label them so that prune can work. So they are only not needed if we have injectLabels set to false.
There was a problem hiding this comment.
If the k *Kubernetes contained the environment name (or baseDir), this would reduce the need to pass it around and remove this extra parameter.
There was a problem hiding this comment.
Yup, add the whole Environment type in there, not just the spec :D
| @@ -70,16 +72,22 @@ func New(s v1alpha1.Spec) (*Kubernetes, error) { | |||
| type ApplyOpts client.ApplyOpts | |||
|
|
|||
| // Apply receives a state object generated using `Reconcile()` and may apply it to the target system | |||
| func (k *Kubernetes) Apply(state manifest.List, opts ApplyOpts) error { | |||
| func (k *Kubernetes) Apply(baseDir string, state manifest.List, opts ApplyOpts) error { | |||
There was a problem hiding this comment.
pkg/kubernetes should not be concerned with directories and such ... that's the concern of pkg/tanka.
For calculating the environment label ... the environments name is available in k *Kubernetes.
There was a problem hiding this comment.
I'll check that. The label needs to be k8s compatible, so must exclude slashes, hence this label has slashes replaced with dots.
There was a problem hiding this comment.
I don't see the environment preset in k *Kubernetes, but... we could put it there.
There was a problem hiding this comment.
Alright, it only includes the spec. I’m fine with adding the whole env in there
pkg/kubernetes/kubernetes.go
Outdated
| if opts.Prune { | ||
| message = `Applying to and pruning namespace '%s' of cluster '%s' at '%s' using context '%s'.` | ||
| } else { | ||
| message = `Applying to namespace '%s' of cluster '%s' at '%s' using context '%s'.` |
There was a problem hiding this comment.
No if else dance please, just default to the one without prune and reassign in case
There was a problem hiding this comment.
huh? Explain? This may be a Golang thing I've not heard.
There was a problem hiding this comment.
message := `Applying to namespace '%s' of cluster '%s' at '%s' using context '%s'.`
if opts.Prune {
message = `Applying to and pruning namespace '%s' of cluster '%s' at '%s' using context '%s'.`
}
pkg/kubernetes/kubernetes.go
Outdated
| @@ -90,7 +98,10 @@ func (k *Kubernetes) Apply(state manifest.List, opts ApplyOpts) error { | |||
| return err | |||
| } | |||
| } | |||
| return k.ctl.Apply(state, client.ApplyOpts(opts)) | |||
|
|
|||
| environmentLabel := TankaEnvironmentLabel + "=" + getEnvironmentLabel(baseDir) | |||
pkg/kubernetes/manifest/manifest.go
Outdated
| @@ -133,6 +133,11 @@ func (m Metadata) Labels() map[string]interface{} { | |||
| return m["labels"].(map[string]interface{}) | |||
| } | |||
|
|
|||
| // SetLabels sets the labels of a manifest | |||
| func (m Metadata) SetLabels(labels map[string]interface{}) { | |||
There was a problem hiding this comment.
No need for that, a map[string]interface{} is a pointer type so you can just assign values on the return value of Labels()
pkg/kubernetes/reconcile.go
Outdated
| @@ -14,23 +14,33 @@ import ( | |||
| "github.com/grafana/tanka/pkg/spec/v1alpha1" | |||
| ) | |||
|
|
|||
| // TankaEnvironmentLabel is the label applied to all resources. Used to identify resources that are | |||
| // eligible for deletion, if not present in the generated set of manifests. | |||
| const TankaEnvironmentLabel = "tanka/environment" | |||
pkg/tanka/tanka.go
Outdated
| @@ -85,3 +85,10 @@ func WithApplyAutoApprove(b bool) Modifier { | |||
| opts.apply.AutoApprove = b | |||
| } | |||
| } | |||
|
|
|||
| // WithPrune invokes `kubectl apply` with the `--prune` flag | |||
| func WithPrune(b bool) Modifier { | |||
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
@sh0rez pointed out that
kubectlsupports--prunewhich does the heavy lifting for us.This PR replaces #117 (which was itself a second try!) and implements #88, using
--prune. Because of that, it has called the feature "prune" to followkubectl.This fix is much more in line with what Tanka has been so far - just passing data around rather than doing heavy lifting.