-
Notifications
You must be signed in to change notification settings - Fork 387
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
[Windows] Update helper.psm1 to set env var for Windows FQDN #2623
Conversation
hack/windows/Helper.psm1
Outdated
$KUBE_DNS_SERVICE_HOST=$(kubectl get service -n kube-system kube-dns -ojsonpath='{.spec.clusterIP}') | ||
$KUBE_DNS_SERVICE_PORT=$(kubectl get service -n kube-system kube-dns -ojsonpath='{.spec.ports[0].port}') |
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.
is -ojsonpath
valid? you use -o jsonpath
above
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 tested on Windows VM and -ojsonpath
did work. But let's make it better.
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.
so now it's -o=jsonpath=
?
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 followed the windows example here: https://kubernetes.io/docs/reference/kubectl/jsonpath/
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.
could we just harmonize and use the same everywhere in the script? I don't really care whether it's -o jsonpath=
or -o=jsonpath=
as long as it works.
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.
Updated. And other -o=jsonpath
usage in the repo.
Codecov Report
@@ Coverage Diff @@
## main #2623 +/- ##
==========================================
- Coverage 60.79% 59.59% -1.20%
==========================================
Files 285 286 +1
Lines 23049 23053 +4
==========================================
- Hits 14012 13738 -274
- Misses 7570 7890 +320
+ Partials 1467 1425 -42
Flags with carried forward coverage won't be shown. Click here to find out more.
|
/test-all |
* When Windows agents are started with script, not in a pod, env var KUBE_DNS_SERVICE_HOST and KUBE_DNS_SERVICE_PORT need to be set. * Harmonize all '-o jsonpath' usage Signed-off-by: Zhecheng Li <lzhecheng@vmware.com>
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.
LGTM
/test-all |
@tnqn @antoninbas could you help merge the PR. Thank you. |
env var KUBE_DNS_SERVICE_HOST and KUBE_DNS_SERVICE_PORT
need to be set.
Signed-off-by: Zhecheng Li lzhecheng@vmware.com