Skip to content

Conversation

@Miciah
Copy link
Contributor

@Miciah Miciah commented Feb 5, 2021

  • go.mod: Bump github.com/openshift/kubernetes.
  • go.sum:
  • vendor/modules.txt: Regenerate.
  • vendor/k8s.io/kubernetes/pkg/proxy/iptables/proxier.go (syncProxyRules): Prefer a local endpoint for the cluster DNS service.

@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 5, 2021
@Miciah Miciah force-pushed the BZ1919737-prefer-local-endpoint-for-cluster-DNS-service branch 2 times, most recently from 8215fa6 to ad49964 Compare February 8, 2021 21:16
@Miciah Miciah changed the title WIP: Prefer local endpoint for cluster DNS service Bug 1919737: Prefer local endpoint for cluster DNS service Feb 8, 2021
@openshift-ci-robot openshift-ci-robot added bugzilla/severity-urgent Referenced Bugzilla bug's severity is urgent for the branch this PR is targeting. bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. and removed do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Feb 8, 2021
@openshift-ci-robot
Copy link
Contributor

@Miciah: This pull request references Bugzilla bug 1919737, which is invalid:

  • expected the bug to target the "4.8.0" release, but it targets "---" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

Details

In response to this:

Bug 1919737: Prefer local endpoint for cluster DNS service

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.

@Miciah
Copy link
Contributor Author

Miciah commented Feb 11, 2021

/bugzilla refresh

@openshift-ci-robot openshift-ci-robot added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Feb 11, 2021
@openshift-ci-robot
Copy link
Contributor

@Miciah: This pull request references Bugzilla bug 1919737, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target release (4.8.0) matches configured target release for branch (4.8.0)
  • bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)
Details

In response to this:

/bugzilla refresh

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.

@openshift-ci-robot openshift-ci-robot removed the bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. label Feb 11, 2021
@Miciah
Copy link
Contributor Author

Miciah commented Feb 11, 2021

/cherry-pick release-4.7

@openshift-cherrypick-robot

@Miciah: once the present PR merges, I will cherry-pick it on top of release-4.7 in a new PR and assign it to you.

Details

In response to this:

/cherry-pick release-4.7

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.

@Miciah
Copy link
Contributor Author

Miciah commented Feb 11, 2021

/cherry-pick release-4.6

@openshift-cherrypick-robot

@Miciah: once the present PR merges, I will cherry-pick it on top of release-4.6 in a new PR and assign it to you.

Details

In response to this:

/cherry-pick release-4.6

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.

if svcNameString == "openshift-dns/dns-default:dns" {
for _, ep := range allEndpoints {
if ep.GetIsLocal() {
allEndpoints = []proxy.Endpoint{ep}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should klog something here (at the same level as other similar messages)

@squeed
Copy link
Contributor

squeed commented Feb 16, 2021

So, this PR needs to be against our fork of Kubernetes. Unfortunately, that is in a bit of a bad state right now.

Openshift 4.8 is currently built against https://github.com/openshift/kubernetes/tree/sdn-4.7-kubernetes-1.20.0-rc.0. What should happen is:

  1. someone else creates openshift/k8s branch sdn-4.8-kubernetes-1.20.0-rc.0 and updates openshift-sdn to use that
  2. You file a PR against that branch in openshift/k8s with this change as a CARRY patch
  3. You file a PR against openshift-sdn with this change fake-vendored in, with a /hold, for CI
  4. Once CI passes, you merge the openshift/k8s PR, update the openshift/sdn pr

Simple! (not)

@danwinship
Copy link
Contributor

someone else creates openshift/k8s branch sdn-4.8-kubernetes-1.20.0-rc.0 and updates openshift-sdn to use that

This PR does not actually have to wait for that. You can just file a PR against sdn-4.7-kubernetes-1.20.0-rc.0 for now, and then when we get around to rebasing for 4.8, your <carry> patch will get copied over

@Miciah Miciah force-pushed the BZ1919737-prefer-local-endpoint-for-cluster-DNS-service branch 2 times, most recently from d2a63d2 to 905c1f4 Compare February 17, 2021 16:10
@Miciah
Copy link
Contributor Author

Miciah commented Feb 17, 2021

Looks like test flakes, nothing that looks DNS- or proxy-related...
/test e2e-aws
/test e2e-aws-upgrade

@Miciah
Copy link
Contributor Author

Miciah commented Feb 17, 2021

/test e2e-aws-upgrade

2 similar comments
@Miciah
Copy link
Contributor Author

Miciah commented Feb 18, 2021

/test e2e-aws-upgrade

@abhat
Copy link
Contributor

abhat commented Feb 18, 2021

/test e2e-aws-upgrade

@Miciah
Copy link
Contributor Author

Miciah commented Feb 18, 2021

The[sig-auth][Feature:SCC][Early] should not have pod creation failures during install failures look like BZ#1913069.
/test e2e-aws-upgrade

@Miciah
Copy link
Contributor Author

Miciah commented Feb 18, 2021

/hold
CI passed; the next step is to merge openshift/kubernetes#574, and then fix the vendor bump in this PR after that.

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 18, 2021
This commit fixes bug 1919737.

https://bugzilla.redhat.com/show_bug.cgi?id=1919737

* go.mod: Bump github.com/openshift/kubernetes.
* go.sum:
* vendor/modules.txt: Regenerate.
* vendor/k8s.io/kubernetes/pkg/proxy/iptables/proxier.go (syncProxyRules):
Prefer a local endpoint for the cluster DNS service.
@Miciah Miciah force-pushed the BZ1919737-prefer-local-endpoint-for-cluster-DNS-service branch from 905c1f4 to 7be0c33 Compare February 18, 2021 21:08
@openshift-ci-robot
Copy link
Contributor

@Miciah: This pull request references Bugzilla bug 1919737, which is valid.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target release (4.8.0) matches configured target release for branch (4.8.0)
  • bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)
Details

In response to this:

Bug 1919737: Prefer local endpoint for cluster DNS service

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.

@danwinship
Copy link
Contributor

/lgtm
assuming it passes ci

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 18, 2021
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: danwinship, Miciah

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

The pull request process is described here

Details 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

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 18, 2021
@Miciah
Copy link
Contributor Author

Miciah commented Feb 19, 2021

Looks like BZ#1913069 again.
/test e2e-gcp

@Miciah
Copy link
Contributor Author

Miciah commented Feb 19, 2021

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 19, 2021
@openshift-merge-robot openshift-merge-robot merged commit 69e55d4 into openshift:master Feb 19, 2021
@openshift-ci-robot
Copy link
Contributor

@Miciah: All pull requests linked via external trackers have merged:

Bugzilla bug 1919737 has been moved to the MODIFIED state.

Details

In response to this:

Bug 1919737: Prefer local endpoint for cluster DNS service

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.

@openshift-cherrypick-robot

@Miciah: new pull request created: #259

Details

In response to this:

/cherry-pick release-4.7

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.

@openshift-cherrypick-robot

@Miciah: #254 failed to apply on top of branch "release-4.6":

Applying: Prefer local endpoint for cluster DNS service
Using index info to reconstruct a base tree...
M	go.mod
M	go.sum
M	vendor/k8s.io/kubernetes/pkg/proxy/iptables/proxier.go
M	vendor/modules.txt
Falling back to patching base and 3-way merge...
Auto-merging vendor/modules.txt
CONFLICT (content): Merge conflict in vendor/modules.txt
Auto-merging vendor/k8s.io/kubernetes/pkg/proxy/iptables/proxier.go
Auto-merging go.sum
CONFLICT (content): Merge conflict in go.sum
Auto-merging go.mod
CONFLICT (content): Merge conflict in go.mod
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Prefer local endpoint for cluster DNS service
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Details

In response to this:

/cherry-pick release-4.6

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. bugzilla/severity-urgent Referenced Bugzilla bug's severity is urgent for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants