-
Notifications
You must be signed in to change notification settings - Fork 240
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
feat: cni refactor for swift v2 #2330
Conversation
28f8d9f
to
c15a2aa
Compare
cni/network/invoker_azure_test.go
Outdated
wantErr bool | ||
}{ | ||
{ | ||
name: "happy add ipv4", | ||
fields: fields{ | ||
plugin: &mockDelegatePlugin{ | ||
add: add{ | ||
resultsIPv4: getResult("10.0.0.1/24"), | ||
resultsIPv4: getSingleResult("10.0.0.1/24"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you move the subnet to a var at the beginning of the file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
cni/network/invoker_azure_test.go
Outdated
@@ -145,16 +152,16 @@ func TestAzureIPAMInvoker_Add(t *testing.T) { | |||
nwCfg: &cni.NetworkConfig{}, | |||
subnetPrefix: getCIDRNotationForAddress("10.0.0.0/24"), | |||
}, | |||
want: getResult("10.0.0.1/24")[0], | |||
want: getResult("10.0.0.1/24"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same thing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
errInvalidDefaultRouting = errors.New("add result requires exactly one interface with default routes") | ||
errInvalidGatewayIP = errors.New("invalid gateway IP") | ||
overlayGatewayV6IP = "fe80::1234:5678:9abc" | ||
watcherPath = "/var/run/azure-vnet/deleteIDs" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need a path for Windows?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not supported atm
@jaer-tsun can we also validate this commit with release pipelines.. check with @jpayne3506 on this |
/azp run CNI Load Test |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run CNI Load Test |
Azure Pipelines successfully started running 1 pipeline(s). |
combining 4 CNI PRs from forked repo
Reason for Change:
CNI changes for Swift 2
Issue Fixed:
Requirements:
Notes: