Skip to content

Commit fec83ae

Browse files
k8s-infra-cherrypick-robotnawazkh
k8s-infra-cherrypick-robot
and
nawazkh
authored
[release-1.19] NIC should be try to fix itself when it is in ProvisioningFailed State (#5575)
* Rebuild nicSpec on provisioning failed status * use anonymous function to get a copy of fakeStaticPrivateIPNICSpec in the test spec --------- Co-authored-by: Nawaz Hussain Khazielakha <[email protected]>
1 parent aaeba8c commit fec83ae

File tree

3 files changed

+167
-4
lines changed

3 files changed

+167
-4
lines changed

‎azure/services/async/async.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ func (s *Service[C, D]) CreateOrUpdateResource(ctx context.Context, spec azure.R
6161
resourceName := spec.ResourceName()
6262
rgName := spec.ResourceGroupName()
6363
futureType := infrav1.PutFuture
64+
log.V(4).Info("CreateOrUpdateResource", "resourceName", resourceName, "rgName", rgName, "futureType", futureType)
6465

6566
// Check if there is an ongoing long-running operation.
6667
resumeToken := ""
@@ -71,6 +72,7 @@ func (s *Service[C, D]) CreateOrUpdateResource(ctx context.Context, spec azure.R
7172
return "", errors.Wrap(err, "could not decode future data, resetting long-running operation state")
7273
}
7374
resumeToken = t
75+
log.V(4).Info("Found a resume token for this long running operation", "resumeToken", resumeToken)
7476
}
7577

7678
// Only when no long running operation is currently in progress do we need to get the parameters.

‎azure/services/networkinterfaces/spec.go

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
"sigs.k8s.io/cluster-api-provider-azure/azure"
2929
"sigs.k8s.io/cluster-api-provider-azure/azure/converters"
3030
"sigs.k8s.io/cluster-api-provider-azure/azure/services/resourceskus"
31+
"sigs.k8s.io/cluster-api-provider-azure/util/tele"
3132
)
3233

3334
// NICSpec defines the specification for a Network Interface.
@@ -80,13 +81,26 @@ func (s *NICSpec) OwnerResourceName() string {
8081
}
8182

8283
// Parameters returns the parameters for the network interface.
83-
func (s *NICSpec) Parameters(_ context.Context, existing interface{}) (parameters interface{}, err error) {
84+
func (s *NICSpec) Parameters(ctx context.Context, existing interface{}) (parameters interface{}, err error) {
85+
_, log, done := tele.StartSpanWithLogger(ctx, "networkinterfaces.NICSpec.Parameters")
86+
defer done()
87+
8488
if existing != nil {
85-
if _, ok := existing.(armnetwork.Interface); !ok {
89+
existingNIC, ok := existing.(armnetwork.Interface)
90+
if !ok {
8691
return nil, errors.Errorf("%T is not an armnetwork.Interface", existing)
8792
}
88-
// network interface already exists
89-
return nil, nil
93+
94+
// If the NIC already exists, return a nil parameters and nil errors to skip the create or update of the already existing NIC.
95+
// If the NIC is in ProvisioningFailed state, we will try to build the parameters of the existing NIC as the provisioning failed does not necessarily mean the NIC is in a bad state.
96+
// Reference: https://learn.microsoft.com/en-us/azure/networking/troubleshoot-failed-state#provisioning-states and
97+
// https://github.com/kubernetes-sigs/cluster-api-provider-azure/issues/5515
98+
if existingNIC.Properties != nil && existingNIC.Properties.ProvisioningState != nil && *existingNIC.Properties.ProvisioningState != armnetwork.ProvisioningStateFailed {
99+
// Return nil for both parameters and error as no changes are needed for the existing resource
100+
// otherwise rebuild the parameters of the existing NIC so that we can patch the ProvisioningState
101+
log.V(4).Info("existing NIC is not in ProvisioningFailed state, returning nil parameters and nil error", "ProvisioningState", *existingNIC.Properties.ProvisioningState)
102+
return nil, nil
103+
}
90104
}
91105

92106
primaryIPConfig := &armnetwork.InterfaceIPConfigurationPropertiesFormat{

‎azure/services/networkinterfaces/spec_test.go

Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -669,6 +669,153 @@ func TestParameters(t *testing.T) {
669669
},
670670
expectedError: "",
671671
},
672+
{
673+
name: "recreate parameters for network interface when Azure provisioning state is Failed",
674+
spec: func() *NICSpec {
675+
s := fakeStaticPrivateIPNICSpec // value‑copy
676+
return &s // pointer to the copy, not the global
677+
}(),
678+
existing: armnetwork.Interface{
679+
ID: ptr.To(""),
680+
Name: ptr.To("my-net-interface"),
681+
Location: ptr.To("fake-location"),
682+
Type: ptr.To("Microsoft.Network/networkInterfaces"),
683+
Properties: &armnetwork.InterfacePropertiesFormat{
684+
ProvisioningState: ptr.To(armnetwork.ProvisioningStateFailed),
685+
},
686+
},
687+
expect: func(g *WithT, result interface{}) {
688+
g.Expect(result).To(BeAssignableToTypeOf(armnetwork.Interface{}))
689+
g.Expect(result.(armnetwork.Interface)).To(Equal(armnetwork.Interface{
690+
Tags: map[string]*string{
691+
"Name": ptr.To("my-net-interface"),
692+
"sigs.k8s.io_cluster-api-provider-azure_cluster_my-cluster": ptr.To("owned"),
693+
},
694+
Location: ptr.To("fake-location"),
695+
Properties: &armnetwork.InterfacePropertiesFormat{
696+
Primary: nil,
697+
EnableAcceleratedNetworking: ptr.To(true),
698+
EnableIPForwarding: ptr.To(false),
699+
DNSSettings: &armnetwork.InterfaceDNSSettings{},
700+
IPConfigurations: []*armnetwork.InterfaceIPConfiguration{
701+
{
702+
Name: ptr.To("pipConfig"),
703+
Properties: &armnetwork.InterfaceIPConfigurationPropertiesFormat{
704+
Primary: ptr.To(true),
705+
LoadBalancerBackendAddressPools: []*armnetwork.BackendAddressPool{{ID: ptr.To("/subscriptions/123/resourceGroups/my-rg/providers/Microsoft.Network/loadBalancers/my-public-lb/backendAddressPools/cluster-name-outboundBackendPool")}},
706+
PrivateIPAllocationMethod: ptr.To(armnetwork.IPAllocationMethodStatic),
707+
PrivateIPAddress: ptr.To("fake.static.ip"),
708+
Subnet: &armnetwork.Subnet{ID: ptr.To("/subscriptions/123/resourceGroups/my-rg/providers/Microsoft.Network/virtualNetworks/my-vnet/subnets/my-subnet")},
709+
},
710+
},
711+
},
712+
},
713+
}))
714+
},
715+
expectedError: "",
716+
},
717+
{
718+
name: "do not recreate parameters for network interface when Azure provisioning state is Deleting",
719+
spec: func() *NICSpec {
720+
s := fakeStaticPrivateIPNICSpec // value‑copy
721+
return &s // pointer to the copy, not the global
722+
}(),
723+
existing: armnetwork.Interface{
724+
ID: ptr.To(""),
725+
Name: ptr.To("my-net-interface"),
726+
Location: ptr.To("fake-location"),
727+
Type: ptr.To("Microsoft.Network/networkInterfaces"),
728+
Properties: &armnetwork.InterfacePropertiesFormat{
729+
ProvisioningState: ptr.To(armnetwork.ProvisioningStateDeleting),
730+
},
731+
},
732+
expect: func(g *WithT, result interface{}) {
733+
g.Expect(result).To(BeNil())
734+
},
735+
expectedError: "",
736+
},
737+
{
738+
name: "do not recreate parameters for network interface when Azure provisioning state is Succeeded",
739+
spec: func() *NICSpec {
740+
s := fakeStaticPrivateIPNICSpec // value‑copy
741+
return &s // pointer to the copy, not the global
742+
}(),
743+
existing: armnetwork.Interface{
744+
ID: ptr.To(""),
745+
Name: ptr.To("my-net-interface"),
746+
Location: ptr.To("fake-location"),
747+
Type: ptr.To("Microsoft.Network/networkInterfaces"),
748+
Properties: &armnetwork.InterfacePropertiesFormat{
749+
ProvisioningState: ptr.To(armnetwork.ProvisioningStateSucceeded),
750+
},
751+
},
752+
expect: func(g *WithT, result interface{}) {
753+
g.Expect(result).To(BeNil())
754+
},
755+
expectedError: "",
756+
},
757+
{
758+
name: "do not recreate parameters for network interface when Azure provisioning state is Updating",
759+
spec: func() *NICSpec {
760+
s := fakeStaticPrivateIPNICSpec // value‑copy
761+
return &s // pointer to the copy, not the global
762+
}(),
763+
existing: armnetwork.Interface{
764+
ID: ptr.To(""),
765+
Name: ptr.To("my-net-interface"),
766+
Location: ptr.To("fake-location"),
767+
Type: ptr.To("Microsoft.Network/networkInterfaces"),
768+
Properties: &armnetwork.InterfacePropertiesFormat{
769+
ProvisioningState: ptr.To(armnetwork.ProvisioningStateUpdating),
770+
},
771+
},
772+
expect: func(g *WithT, result interface{}) {
773+
g.Expect(result).To(BeNil())
774+
},
775+
expectedError: "",
776+
},
777+
{
778+
name: "recreate parameters for network interface when Azure provisioning state nil",
779+
spec: func() *NICSpec {
780+
s := fakeStaticPrivateIPNICSpec // value‑copy
781+
return &s // pointer to the copy, not the global
782+
}(),
783+
existing: armnetwork.Interface{
784+
ID: ptr.To(""),
785+
Name: ptr.To("my-net-interface"),
786+
Location: ptr.To("fake-location"),
787+
Type: ptr.To("Microsoft.Network/networkInterfaces"),
788+
},
789+
expect: func(g *WithT, result interface{}) {
790+
g.Expect(result).To(BeAssignableToTypeOf(armnetwork.Interface{}))
791+
g.Expect(result.(armnetwork.Interface)).To(Equal(armnetwork.Interface{
792+
Tags: map[string]*string{
793+
"Name": ptr.To("my-net-interface"),
794+
"sigs.k8s.io_cluster-api-provider-azure_cluster_my-cluster": ptr.To("owned"),
795+
},
796+
Location: ptr.To("fake-location"),
797+
Properties: &armnetwork.InterfacePropertiesFormat{
798+
Primary: nil,
799+
EnableAcceleratedNetworking: ptr.To(true),
800+
EnableIPForwarding: ptr.To(false),
801+
DNSSettings: &armnetwork.InterfaceDNSSettings{},
802+
IPConfigurations: []*armnetwork.InterfaceIPConfiguration{
803+
{
804+
Name: ptr.To("pipConfig"),
805+
Properties: &armnetwork.InterfaceIPConfigurationPropertiesFormat{
806+
Primary: ptr.To(true),
807+
LoadBalancerBackendAddressPools: []*armnetwork.BackendAddressPool{{ID: ptr.To("/subscriptions/123/resourceGroups/my-rg/providers/Microsoft.Network/loadBalancers/my-public-lb/backendAddressPools/cluster-name-outboundBackendPool")}},
808+
PrivateIPAllocationMethod: ptr.To(armnetwork.IPAllocationMethodStatic),
809+
PrivateIPAddress: ptr.To("fake.static.ip"),
810+
Subnet: &armnetwork.Subnet{ID: ptr.To("/subscriptions/123/resourceGroups/my-rg/providers/Microsoft.Network/virtualNetworks/my-vnet/subnets/my-subnet")},
811+
},
812+
},
813+
},
814+
},
815+
}))
816+
},
817+
expectedError: "",
818+
},
672819
}
673820
format.MaxLength = 10000
674821
for _, tc := range testcases {

0 commit comments

Comments
 (0)