Skip to content
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

Partial cherry pick of #85396: Move hostdns.conf out of CNI config directory. #85823

Conversation

@Random-Liu
Copy link
Member

Random-Liu commented Dec 2, 2019

Partial cherry pick of #85396 on release-1.17.

#85396: Move hostdns.conf out of CNI config directory.

For details on the cherry pick process, see the cherry pick requests page.

Fix hostdns.conf parsing error in kubelet log on GCE windows node.
@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented Dec 2, 2019

@Random-Liu: This cherry pick PR is for a release branch and has not yet been approved by the Patch Release Team.
Adding the do-not-merge/cherry-pick-not-approved label.

To merge this cherry pick, please ping the kubernetes/patch-release-team in a comment, after it has been approved by the relevant OWNERS.
For details on the patch release process and schedule, see the Patch Releases page.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot requested review from fejta and mtaufen Dec 2, 2019
@Random-Liu Random-Liu force-pushed the Random-Liu:automated-cherry-pick-of-#85396-upstream-release-1.17 branch from b1e685f to 330897a Dec 2, 2019
@Random-Liu Random-Liu changed the title Automated cherry pick of #85396: Add containerd windows support on GCE for test. Partial cherry pick of #85396: Move hostdns.conf out of CNI config directory. Dec 2, 2019
@Random-Liu Random-Liu added the kind/bug label Dec 2, 2019
@k8s-ci-robot k8s-ci-robot removed the needs-kind label Dec 2, 2019
@Random-Liu

This comment has been minimized.

Copy link
Member Author

Random-Liu commented Dec 2, 2019

@k8s-ci-robot k8s-ci-robot requested review from yliaog and pjh Dec 2, 2019
@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented Dec 2, 2019

@Random-Liu: GitHub didn't allow me to request PR reviews from the following users: lzang.

Note that only kubernetes members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @yliaog @lzang @pjh

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@Random-Liu

This comment has been minimized.

Copy link
Member Author

Random-Liu commented Dec 2, 2019

/test pull-kubernetes-e2e-windows-gce

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented Dec 2, 2019

@Random-Liu: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-kubernetes-cross b1e685f link /test pull-kubernetes-cross

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@@ -873,7 +873,7 @@ function construct-windows-kubelet-flags {
flags+=" --logtostderr=false"

# Configure the file path for host dns configuration
flags+=" --resolv-conf=${WINDOWS_CNI_CONFIG_DIR}\hostdns.conf"
flags+=" --resolv-conf=${WINDOWS_CNI_DIR}\hostdns.conf"

This comment has been minimized.

Copy link
@yliaog

yliaog Dec 2, 2019

Contributor

is it cni at all? it's about DNS.

This comment has been minimized.

Copy link
@Random-Liu

Random-Liu Dec 2, 2019

Author Member

It seems not a CNI thing, it is just network related.

We can organize the directory structure better, currently the cni directory is the only place containing some network stuff. :)

This comment has been minimized.

Copy link
@yliaog

yliaog Dec 2, 2019

Contributor

ok, let's organize it later. thanks.

This comment has been minimized.

Copy link
@lzang

lzang Dec 3, 2019

Contributor

For windows, dns config is through cni. We don't have a network directory and would be nice to have one.
/lgtm

@yliaog

This comment has been minimized.

Copy link
Contributor

yliaog commented Dec 2, 2019

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm label Dec 2, 2019
@pjh

This comment has been minimized.

Copy link
Contributor

pjh commented Dec 3, 2019

/lgtm

@yujuhong

This comment has been minimized.

Copy link
Member

yujuhong commented Dec 3, 2019

/approve

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented Dec 3, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Random-Liu, yliaog, yujuhong

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented Dec 3, 2019

@lzang: changing LGTM is restricted to collaborators

In response to this:

For windows, dns config is through cni. We don't have a network directory and would be nice to have one.
/lgtm

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@feiskyer

This comment has been minimized.

Copy link
Member

feiskyer commented Dec 4, 2019

/retest

@justaugustus

This comment has been minimized.

Copy link
Member

justaugustus commented Dec 5, 2019

We're planning to hold this until 1.17.1 as the approvals missed the cherry pick deadline for 1.17.0.

@yliaog / @Random-Liu / @BenTheElder / @pjh / @yujuhong -- Would there be any reason to pull it in for 1.17.0 instead? Severity/risk/urgency?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.