Filter namespaces by labels #46
Conversation
|
Any feedback on this? |
|
@linki Let me know if there's anything I can do to help this progress. |
| pods []map[string]string | ||
| }{ | ||
| {"", []map[string]string{foo, bar}}, | ||
| {"doesnotexist", []map[string]string{foo, bar}}, |
There was a problem hiding this comment.
I would expect this to not find any pods as there's no namespace that satisfies this label selector.
| @@ -7,6 +7,8 @@ import ( | |||
|
|
|||
| log "github.com/sirupsen/logrus" | |||
|
|
|||
| "strings" | |||
There was a problem hiding this comment.
Stdlib imports live in the very first section, i.e. next to errors, fmt, etc.
| // Append additional namespaces filters | ||
| reqs, _ := namespaceLabel.Requirements() | ||
| for _, req := range reqs { | ||
| c.Namespaces = c.Namespaces.Add(req) |
There was a problem hiding this comment.
I have to think about this. I was thinking we could do this in the initializer (resolving the namespace label selector and then adding the matched namespaces to the name-based namespace selector).
But then, having it here makes much more sense as the label selector is re-evaluated at every loop (namespaces could have been created, labels on namespaces can change to opt-in/out them etc.).
However, modifying the chaoskube-instance global variable here might not have the desired effect in this case.
|
Looks awesome, thanks @swestcott! I have a remark regarding how the final namespace list is calculated, let me know if you have thoughts about it. Of the top of my head, I think we need to add the namespaces found by label selectors to the final namespace selector list via a local variable, so that we don't ever grow this list. |
|
Follow up: swestcott#1 |
|
Rebased on the latest master and merged in #134. Thanks @swestcott |
PR for #44 to add a new
--namespace-labelsoption.I'm still fairly new to Go, so all feedback is welcome :)