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.

Sign up for GitHub

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

Jump to bottom

Automated cherry pick of #78012: Upgrade Azure network API version to 2018-07-01 #78065

Merged
merged 2 commits into from
Jun 4, 2019
Merged

Automated cherry pick of #78012: Upgrade Azure network API version to 2018-07-01 #78065

merged 2 commits into from
Jun 4, 2019

Conversation

feiskyer
Copy link
Member

Cherry pick of #78012 on release-1.13.

#78012: Upgrade Azure network API version to 2018-07-01

@k8s-ci-robot k8s-ci-robot added do-not-merge/cherry-pick-not-approved Indicates that a PR is not yet approved to merge into a release branch. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-kind Indicates a PR lacks a `kind/foo` label and requires one. needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. sig/cloud-provider Categorizes an issue or PR as relevant to SIG Cloud Provider. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels May 18, 2019
@feiskyer
Copy link
Member Author

/sig azure
/kind bug
/priority critical-urgent

/assign @justaugustus @dims @andyzhangx

@k8s-ci-robot k8s-ci-robot added sig/azure kind/bug Categorizes issue or PR as related to a bug. priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. and removed needs-kind Indicates a PR lacks a `kind/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels May 18, 2019
@justaugustus
Copy link
Member

justaugustus commented May 19, 2019 •
edited

For SIG Azure & SIG Release:
/lgtm
/approve

though I will echo the sentiment from #78012 that we need to ensure we make a concrete plan to minimize the size of our cherry picks moving forward.

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 19, 2019
@feiskyer
Copy link
Member Author

feiskyer commented May 20, 2019 •
edited

@dims Could you help to approve the godep updates?

@dims
Copy link
Member

dims commented May 20, 2019

/approve

@tpepper
Copy link
Member

tpepper commented May 21, 2019 •
edited

/hold

Why is this not a feature? I get the rationale in #78065 for master, but here I'm not as convinced it applies.

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 21, 2019
@tpepper tpepper mentioned this pull request May 21, 2019
Merged
@feiskyer
Copy link
Member Author

Why is this not a feature? I get the rationale in #78065 for master, but here I'm not as convinced it applies.

it's a bug fix: without this, if users have configure EnableTcpReset on their loadbalancer, then all Kubernetes LoadBalancer services would be stuck in pending state.

@justaugustus
Copy link
Member

I'm okay with the rationale here, so I'm lifting the hold. This should enable much smaller, concise bug fixes for Azure services once merged.
/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 27, 2019
@justaugustus
Copy link
Member

Looks like @dims is not a dep approver on release-1.13, so this still needs approval:
/assign @cblecker @thockin @sttts

@cblecker
Copy link
Member

We really need to get this Azure cloud provider out of tree ASAP. The fact that a load balancer bug fix causes a 19k LOC vendor change isn't sustainable.

Need to review this closely..

/hold

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 27, 2019
@feiskyer
Copy link
Member Author

We really need to get this Azure cloud provider out of tree ASAP. The fact that a load balancer bug fix causes a 19k LOC vendor change isn't sustainable.

There's still much work to do before out-of-tree cloud provider. And most of those changes are vendor update, so I think it's ok for cherry picking.

@cblecker
Copy link
Member

Still thinking about this, but I'm gonna take a look after 1.15's code freeze on Thursday. So either Friday or early next week. Thanks for your patience, just need to prioritize.

@thockin thockin removed their assignment May 31, 2019
@feiskyer
Copy link
Member Author

ping @cblecker

@cblecker
Copy link
Member

cblecker commented Jun 2, 2019

/assign
/approve
/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 2, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cblecker, dims, feiskyer, justaugustus

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 k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 2, 2019
@tpepper
Copy link
Member

tpepper commented Jun 4, 2019

I'm uncomfortable with this change similar to Jordan's comment at #78066 (comment) but also for the reasons listed there (isolation to Azure and their emphasis on this as critical fix) I'm going to grudgingly approve this.

@tpepper tpepper added the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label Jun 4, 2019
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/cherry-pick-not-approved Indicates that a PR is not yet approved to merge into a release branch. label Jun 4, 2019
@tpepper
Copy link
Member

tpepper commented Jun 4, 2019

/retest

@k8s-ci-robot k8s-ci-robot merged commit 109996e into kubernetes:release-1.13 Jun 4, 2019
@feiskyer feiskyer deleted the az-network-2018-07-01-1.13 branch June 6, 2019 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brendandburns brendandburns Awaiting requested review from brendandburns

@lavalamp lavalamp Awaiting requested review from lavalamp

Assignees

@dims dims

@justaugustus justaugustus

@sttts sttts

@cblecker cblecker

@andyzhangx andyzhangx

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/bug Categorizes issue or PR as related to a bug. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. sig/cloud-provider Categorizes an issue or PR as relevant to SIG Cloud Provider. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
9 participants
@feiskyer @justaugustus @dims @tpepper @cblecker @k8s-ci-robot @sttts @andyzhangx @thockin