egctl: add label selector for egctl config dump of Pod #1385
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1385 +/- ##
==========================================
- Coverage 62.34% 62.31% -0.03%
==========================================
Files 78 78
Lines 10973 11024 +51
==========================================
+ Hits 6841 6870 +29
- Misses 3680 3697 +17
- Partials 452 457 +5
|
internal/cmd/egctl/config.go
Outdated
| if len(podList.Items) == 0 { | ||
| return nil, fmt.Errorf("no Pods found for label selectors %+v", labelSelectors) | ||
| } | ||
| if len(podList.Items) > 1 { |
There was a problem hiding this comment.
curious why the 1 pod limitation is added ?
There was a problem hiding this comment.
I was not clear how do we output configs of multiple Pods. right now config of one pod is a singular json (lets say {....}), so for the output of multiple Pods could we output it as follows?
{
pod-name-1: {....},
pod-name-2: {....},
}
because the config output doesn't have a pod name reference, so how do we differentiate between multiple dumps.
So I just limited it to 1 for now.
There was a problem hiding this comment.
ah got it, yah a map works, thats what we recently did with translate
could be done here or lets raise a issue to this can be tracked
cc @zirain
There was a problem hiding this comment.
wouldn't <namespace>-<name> be tricky to interpret given that both of them can have - in their names?
There was a problem hiding this comment.
I think we need some spec around these usecases, maybe something like this?
{
ns-1: {
pod-name-1: {....},
pod-name-2: {....},
},
}
There was a problem hiding this comment.
we are using ns-name across the project as a key for config e.g
gateway/internal/gatewayapi/helpers.go
Line 388 in f998eac
ns-1: {
pod-name-1: {....},
gets tricky because it makes the definition more Kubernetesy which we should ideally avoid, if we want to support other environments
There was a problem hiding this comment.
thinking again, something like
ns-1: {
pod-name-1: {....},
is more readable for the end user, which might make sense
There was a problem hiding this comment.
thinking again, something like
ns-1: { pod-name-1: {....},is more readable for the end user, which might make sense
+1 for this style for visibility
There was a problem hiding this comment.
thinking again, something like
ns-1: { pod-name-1: {....},is more readable for the end user, which might make sense
+1 for this
There was a problem hiding this comment.
nice. will output in this format then, and update the PR
Signed-off-by: Shubham Chauhan <shubham@tetrate.io>
Signed-off-by: Shubham Chauhan <shubham@tetrate.io>
Signed-off-by: Shubham Chauhan <shubham@tetrate.io>
Signed-off-by: Shubham Chauhan <shubham@tetrate.io>
chauhanshubham
force-pushed
the
egctl-label-selector
branch
from
ca0bcf4
to
5b3c662
Compare
Signed-off-by: Shubham Chauhan <shubham@tetrate.io>
While using
egctl config envoy-proxyCLI, there is an additional stepto fetch the Pod name which can then be used in the egctl command.
This commit adds support to select a Pod based on label selectors, so that
it's easier to query for the envoy-proxy config of that Pod with something like
where we can use the envoy gateway owner labels to fetch the Pod. This
eliminates the need to get the Pod name specifically.
Right now this commit adds a limitation that the label selectors must select one and only one Pod.
If there are more than one pods selected, there is a requirement to change the output format,
something like
which is still open for discussion, as part of issue #1069