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

fix: determine the correct ip config based on ip family #93043

Merged
merged 1 commit into from Jul 17, 2020
Merged

fix: determine the correct ip config based on ip family #93043

merged 1 commit into from Jul 17, 2020

Conversation

aramase
Copy link
Member

@aramase aramase commented Jul 13, 2020 •
edited

What type of PR is this?
/kind bug

What this PR does / why we need it:
Determines the correct IP config based on clusterIP family before updating backend pool for VMSS

Which issue(s) this PR fixes:

Fixes #93085

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

fix: determine the correct ip config based on ip family

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

NONE

/area provider/azure
/priority important-soon

/assign @feiskyer @khenidak

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/bug Categorizes issue or PR as related to a bug. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. area/provider/azure Issues or PRs related to azure provider priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Jul 13, 2020
@k8s-ci-robot k8s-ci-robot added area/cloudprovider 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 Jul 13, 2020
@aramase aramase mentioned this pull request Jul 13, 2020
Closed
@aramase
Copy link
Member Author

aramase commented Jul 14, 2020

/retest

andyzhangx
andyzhangx reviewed Jul 14, 2020
View reviewed changes
staging/src/k8s.io/legacy-cloud-providers/azure/azure_vmss.go
if err != nil {
return err
var primaryIPConfig *compute.VirtualMachineScaleSetIPConfiguration
ipv6 := utilnet.IsIPv6String(service.Spec.ClusterIP)
Copy link
Member

Choose a reason for hiding this comment

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

more comments on this? why ipv6 and ipv4 get from different config?

Copy link
Member

Choose a reason for hiding this comment

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

and there must be a issue related to this PR if it's going to cherry-pick to old release

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you. Added a comment and opened an issue #93085

andyzhangx
andyzhangx approved these changes Jul 15, 2020
View reviewed changes
Copy link
Member

@andyzhangx andyzhangx left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@andyzhangx
Copy link
Member

/retest

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 15, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andyzhangx, aramase

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 Jul 15, 2020
@fejta-bot
Copy link

/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.

1 similar comment
@fejta-bot
Copy link

/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.

@andyzhangx
Copy link
Member

/retest

1 similar comment
@aramase
Copy link
Member Author

aramase commented Jul 15, 2020

/retest

@fejta-bot
Copy link

/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
Copy link

/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.

4 similar comments
@fejta-bot
Copy link

/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
Copy link

/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
Copy link

/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
Copy link

/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.

@aramase
Copy link
Member Author

aramase commented Jul 16, 2020

@andyzhangx Do you know what the issue is with pull-kubernetes-e2e-azure-disk-windows and pull-kubernetes-e2e-azure-file-windows?

Is this PR - #93052 expected to fix the issue?

@fejta-bot
Copy link

/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.

@andyzhangx
Copy link
Member

@andyzhangx Do you know what the issue is with pull-kubernetes-e2e-azure-disk-windows and pull-kubernetes-e2e-azure-file-windows?

Is this PR - #93052 expected to fix the issue?

It's due to this windows bug: #93063

@feiskyer
Copy link
Member

/milestone v1.19

@k8s-ci-robot k8s-ci-robot added this to the v1.19 milestone Jul 17, 2020
@andyzhangx
Copy link
Member

andyzhangx commented Jul 17, 2020 •
edited

BTW, the windows failing tests are non-blocking, let's merge this PR first, thanks.

@fejta-bot
Copy link

/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.

2 similar comments
@fejta-bot
Copy link

/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
Copy link

/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.

@aramase
Copy link
Member Author

aramase commented Jul 17, 2020

/test pull-kubernetes-e2e-kind

@fejta-bot
Copy link

/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
Copy link
Contributor

@aramase: The following tests failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
pull-kubernetes-e2e-azure-file-windows b1f0e98 link /test pull-kubernetes-e2e-azure-file-windows
pull-kubernetes-e2e-azure-disk-windows b1f0e98 link /test pull-kubernetes-e2e-azure-disk-windows

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.

@k8s-ci-robot k8s-ci-robot merged commit 0a7050f into kubernetes:master Jul 17, 2020
@aramase aramase deleted the vmss-dualstack-ipconfig branch July 17, 2020 22:04
@ArchangelSDY
Copy link
Contributor

Hit this issue in v1.18. Has this been cherry-picked to previous releases? @aramase @andyzhangx @feiskyer

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

@andyzhangx andyzhangx andyzhangx approved these changes

@justaugustus justaugustus Awaiting requested review from justaugustus

@karataliu karataliu Awaiting requested review from karataliu

Assignees

@feiskyer feiskyer

@andyzhangx andyzhangx

@khenidak khenidak

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/cloudprovider area/provider/azure Issues or PRs related to azure provider 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/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/cloud-provider Categorizes an issue or PR as relevant to SIG Cloud Provider. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
v1.19
Development

Successfully merging this pull request may close these issues.

failed to add backend pool for VMSS for IPv6 service
7 participants
@aramase @andyzhangx @k8s-ci-robot @fejta-bot @feiskyer @ArchangelSDY @khenidak