Specify a DialContext in storage plugin clients #91785

mattcary · 2020-06-04T18:25:39Z

Adds filtering of hosts to DialContexts. the provided DialContext wraps existing
clients' DialContext in an attempt to preserve any existing timeout
configuration. In some cases, we may replace infinite timeouts with golang
defaults.

scaleio: tcp connect/keepalive values changed from 0/15 to 30/30
storageos: no change

The added unit tests will fail to compile if the vendor patches are
accidentally reverted.

gluster and quobyte should be updated, but that is blocked on upstream changes on their drivers.

/kind bug
Which issue(s) this PR fixes:
#91542

kube-controller-manager: volume plugins can be restricted from contacting local and loopback addresses by setting `--volume-host-allow-local-loopback=false`, or from contacting specific CIDR ranges by setting `--volume-host-cidr-denylist` (for example, `--volume-host-cidr-denylist=127.0.0.1/28,feed::/16`)

k8s-ci-robot · 2020-06-04T18:25:47Z

Welcome @mattcary!

It looks like this is your first PR to kubernetes/kubernetes 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/kubernetes has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2020-06-04T18:25:48Z

Hi @mattcary. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

mattcary · 2020-06-04T18:26:32Z

/cc msau
/cc tallclair

k8s-ci-robot · 2020-06-04T18:26:34Z

@mattcary: GitHub didn't allow me to request PR reviews from the following users: msau.

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

In response to this:

/cc msau
/cc tallclair

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.

mattcary · 2020-06-04T18:28:52Z

Note that the current behavior of this patch is to always deny dialing localhost and similar addresses (as defined by proxy/util:IsProxyableIP).

It's not clear to me that's the behavior we want and I'd appreciate any opinions from the community.

(in addition to opinions about everything else in the CL :)

msau42 · 2020-06-04T18:34:14Z

/assign @misterikkit @jsafrane @gnufied
/ok-to-test

msau42 · 2020-06-04T18:32:33Z

pkg/volume/BUILD

@@ -54,6 +55,7 @@ go_library(
 go_test(
    name = "go_default_test",
    srcs = [
+        "gke_patch_test.go",


Remove this

done.

Jonathon suggested a different test to replace it but I'm not clear on what we're shooting for here.

msau42 · 2020-06-04T18:33:32Z

vendor/github.com/heketi/heketi/client/api/go-client/client.go

@@ -14,13 +14,15 @@ package client

 import (
 	"bytes"
+	"context"


We need to update vendor with the upstream changes instead of changing vendor directly. cc @misterikkit

Thanks Michelle & Jonathon.

I'm not familiar with how to update vendor/, any pointers would be appreciated (here or offline).

msau42 · 2020-09-03T00:42:12Z

/milestone v1.20

The default behavior is unchanged, so this can be considered low risk.

justaugustus · 2020-09-03T03:52:06Z

Looks like /hack/update-vendor.sh needs to be run on this one.

fejta-bot · 2020-09-03T06:03:36Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

fejta-bot · 2020-09-03T08:51:40Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

fejta-bot · 2020-09-03T12:00:35Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

fejta-bot · 2020-09-03T14:48:33Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

mattcary · 2020-09-03T15:51:28Z

/test pull-kubernetes-verify

Looks like /hack/update-vendor.sh needs to be run on this one.

neither udpate-bazal nor update-vendor doesn't change anything on my local client. verify-bazel passes locally for me. Maybe a flake?

mattcary · 2020-09-03T16:26:20Z

Trying a rebase.

cheftako · 2020-09-14T20:56:16Z

Looks like it needs ./hack/update-bazel.sh to be run again.

mattcary · 2020-09-14T22:01:11Z

I've run update-bazel and verify-bazel many times after many different rebasing tries on two different machines from two different checkouts, and neither reports any updates or changes my commits. verify-bazel runs successfully. I've tried manually patching in the udpate-bazel output from the presubmit queue, but that just fails the bazel build.

I've tried various other hack/update-* or hack/verify-* and they report that everything is working.

I think I'll need to make a totally new kubernetes checkout with a new go install, but it will take me a little while to get the time to do that.

The provided DialContext wraps existing clients' DialContext in an attempt to preserve any existing timeout configuration. In some cases, we may replace infinite timeouts with golang defaults. - scaleio: tcp connect/keepalive values changed from 0/15 to 30/30 - storageos: no change

Change-Id: Iebc99ee13587f0cd4c43ab85c7295d458d679d1e

mattcary · 2020-09-18T15:54:08Z

Hooray! update-bazel finally did something. Is it possible that one has to have a bazel-build succeed or somehow get sufficiently far before verify-bazel/update-bazel are effective? At any rate it seems happy now.

It made lots of updates in vendor/, I'm not sure if that's expected/desired?

msau42 · 2020-09-18T16:25:58Z

/lgtm
Vendor changes seem reasonable, just storageos libraries

/retest

fejta-bot · 2020-09-18T19:48:14Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jun 4, 2020

k8s-ci-robot requested a review from tallclair June 4, 2020 18:26

k8s-ci-robot requested review from caseydavenport and deads2k June 4, 2020 18:27

k8s-ci-robot assigned gnufied, jsafrane and misterikkit Jun 4, 2020

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jun 4, 2020

msau42 reviewed Jun 4, 2020

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 3, 2020

k8s-ci-robot added this to the v1.20 milestone Sep 3, 2020

mattcary force-pushed the filtereddial branch from fc46af7 to e6dee1f Compare September 3, 2020 16:25

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 3, 2020

mattcary force-pushed the filtereddial branch 3 times, most recently from ffca8eb to 11dbc69 Compare September 11, 2020 20:19

mattcary added 3 commits September 18, 2020 00:07

update storageos vendor for FilteredDial change

74dbf27

update-bazel

299a296

Change-Id: Iebc99ee13587f0cd4c43ab85c7295d458d679d1e

mattcary force-pushed the filtereddial branch from 11dbc69 to 299a296 Compare September 18, 2020 00:45

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 18, 2020

k8s-ci-robot merged commit b86e725 into kubernetes:master Sep 18, 2020

github-actions bot mentioned this pull request Sep 22, 2020

Week Ending September 20, 2020 dev-obs/actus#229

Open

misterikkit mentioned this pull request Sep 30, 2020

storage: Use FilteredDialContext in quobyte client #95206

Merged

liggitt mentioned this pull request Jan 26, 2021

Resolve IP addresses of host-only in filtered dialer #98436

Merged

danwinship mentioned this pull request May 18, 2023

helper function for unused storage feature in pkg/proxy/util #118120

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Specify a DialContext in storage plugin clients #91785

Specify a DialContext in storage plugin clients #91785

mattcary commented Jun 4, 2020 •

edited

Loading

k8s-ci-robot commented Jun 4, 2020

k8s-ci-robot commented Jun 4, 2020

mattcary commented Jun 4, 2020

k8s-ci-robot commented Jun 4, 2020

mattcary commented Jun 4, 2020

msau42 commented Jun 4, 2020

msau42 Jun 4, 2020

mattcary Jun 4, 2020

msau42 Jun 4, 2020

mattcary Jun 4, 2020

msau42 commented Sep 3, 2020

justaugustus commented Sep 3, 2020

fejta-bot commented Sep 3, 2020

fejta-bot commented Sep 3, 2020

fejta-bot commented Sep 3, 2020

fejta-bot commented Sep 3, 2020

mattcary commented Sep 3, 2020

mattcary commented Sep 3, 2020

cheftako commented Sep 14, 2020

mattcary commented Sep 14, 2020

mattcary commented Sep 18, 2020

msau42 commented Sep 18, 2020

fejta-bot commented Sep 18, 2020

Specify a DialContext in storage plugin clients #91785

Specify a DialContext in storage plugin clients #91785

Conversation

mattcary commented Jun 4, 2020 • edited Loading

k8s-ci-robot commented Jun 4, 2020

k8s-ci-robot commented Jun 4, 2020

mattcary commented Jun 4, 2020

k8s-ci-robot commented Jun 4, 2020

mattcary commented Jun 4, 2020

msau42 commented Jun 4, 2020

msau42 Jun 4, 2020

Choose a reason for hiding this comment

mattcary Jun 4, 2020

Choose a reason for hiding this comment

msau42 Jun 4, 2020

Choose a reason for hiding this comment

mattcary Jun 4, 2020

Choose a reason for hiding this comment

msau42 commented Sep 3, 2020

justaugustus commented Sep 3, 2020

fejta-bot commented Sep 3, 2020

fejta-bot commented Sep 3, 2020

fejta-bot commented Sep 3, 2020

fejta-bot commented Sep 3, 2020

mattcary commented Sep 3, 2020

mattcary commented Sep 3, 2020

cheftako commented Sep 14, 2020

mattcary commented Sep 14, 2020

mattcary commented Sep 18, 2020

msau42 commented Sep 18, 2020

fejta-bot commented Sep 18, 2020

mattcary commented Jun 4, 2020 •

edited

Loading