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.

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

fix: bugfixes and release condition update #634

Merged
merged 9 commits into from
Aug 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/chart.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ name: publish_helm_chart
on:
pull_request:
branches:
- release-**
- release-**-**
- master
types: [ closed ]

Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/create-release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: Create release
on:
pull_request:
branches:
- release-**
- release-**-**
types: [ closed ]

permissions:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ on:
pull_request:
branches:
- master
- release-**
- release-**-**
paths-ignore: ['docs/**', '**.md', '**.mdx', '**.png', '**.jpg']

env:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ on:
pull_request:
branches:
- master
- release-**
- release-**-**
paths-ignore: ['docs/**', '**.md', '**.mdx', '**.png', '**.jpg']
push:
branches: [master]
Expand Down
4 changes: 2 additions & 2 deletions e2e/initvalidation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import (
)

func TestInitVaidationContainer(t *testing.T) {
cmd := kubectl("get", "pod", "-l", "app=aci-connector-linux", "-n", "kube-system", "-o", "jsonpath={.items[1].status.phase}")
cmd := kubectl("get", "pod", "-l", "app=virtual-kubelet-azure-aci", "-n", "vk-azure-aci", "-o", "jsonpath={.items[0].status.phase}")

out, err := cmd.CombinedOutput()
if err != nil {
Expand All @@ -16,7 +16,7 @@ func TestInitVaidationContainer(t *testing.T) {
t.Fatal("pod is not in running state")
}

cmd = kubectl("logs", "-l", "app=aci-connector-linux", "-c", "init-validation", "-n", "kube-system", "--tail=1")
cmd = kubectl("logs", "-l", "app=virtual-kubelet-azure-aci", "-c", "init-validation", "-n", "vk-azure-aci", "--tail=1")
logOut, logErr := cmd.CombinedOutput()
if logErr != nil {
t.Fatal(string(out))
Expand Down
60 changes: 27 additions & 33 deletions pkg/network/aci_network.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,20 +133,15 @@ func (pn *ProviderNetwork) setupNetwork(ctx context.Context, azConfig *auth.Conf
return err
}

var updateSubnet, createNewSubnet bool
if currentSubnet == (aznetworkv2.Subnet{}) {
createNewSubnet = true
} else {
// check if the current subnet is valid or if we need to create a new subnet
updateSubnet, err = pn.shouldCreateSubnet(currentSubnet)
if err != nil {
return err
}
// check if the current subnet is valid or if we need to create a new subnet
createOrUpdateSubnet, err := pn.shouldCreateOrUpdateSubnet(currentSubnet)
if err != nil {
return err
}

if createNewSubnet || updateSubnet {
// decide whether to create a new subnet or update the existing one based on createNewSubnet bool
err2 := pn.CreateOrUpdateACISubnet(ctx, subnetsClient, createNewSubnet)
if createOrUpdateSubnet {
// decide whether to create a new subnet or update the existing one based currentSubnet
err2 := pn.CreateOrUpdateACISubnet(ctx, subnetsClient, currentSubnet)
if err2 != nil {
return err2
}
Expand All @@ -156,7 +151,7 @@ func (pn *ProviderNetwork) setupNetwork(ctx context.Context, azConfig *auth.Conf
return nil
}

func (pn *ProviderNetwork) shouldCreateSubnet(currentSubnet aznetworkv2.Subnet) (bool, error) {
func (pn *ProviderNetwork) shouldCreateOrUpdateSubnet(currentSubnet aznetworkv2.Subnet) (bool, error) {
createSubnet := true
//check if addressPrefix has been set
if currentSubnet.Properties.AddressPrefix != nil && len(*currentSubnet.Properties.AddressPrefix) > 0 {
Expand Down Expand Up @@ -256,36 +251,35 @@ func (pn *ProviderNetwork) GetSubnetClient(ctx context.Context, azConfig *auth.C
return subnetsClient, nil
}

func (pn *ProviderNetwork) CreateOrUpdateACISubnet(ctx context.Context, subnetsClient *aznetworkv2.SubnetsClient, isCreate bool) error {
func (pn *ProviderNetwork) CreateOrUpdateACISubnet(ctx context.Context, subnetsClient *aznetworkv2.SubnetsClient, currentSubnet aznetworkv2.Subnet) error {
logger := log.G(ctx).WithField("method", "CreateOrUpdateACISubnet")
ctx, span := trace.StartSpan(ctx, "network.CreateOrUpdateACISubnet")
defer span.End()

action := "updating"

subnet := aznetworkv2.Subnet{
Name: &pn.SubnetName,
Properties: &aznetworkv2.SubnetPropertiesFormat{
Delegations: []*aznetworkv2.Delegation{
{
Name: &delegationName,
Properties: &aznetworkv2.ServiceDelegationPropertiesFormat{
ServiceName: &serviceName,
Actions: []*string{&subnetAction},
},
},
subnet := currentSubnet
if currentSubnet == (aznetworkv2.Subnet{}) {
action = "creating"

// only set the address prefix if we are creating a new subnet
subnet = aznetworkv2.Subnet{
Name: &pn.SubnetName,
Properties: &aznetworkv2.SubnetPropertiesFormat{
AddressPrefix: &pn.SubnetCIDR,
},
},
}
}

// only set the address prefix and prefixes if we are creating a new subnet
if isCreate {
subnet.Properties.AddressPrefix = &pn.SubnetCIDR
smritidahal653 marked this conversation as resolved.
Show resolved Hide resolved
subnet.Properties.AddressPrefixes = []*string{
&pn.SubnetCIDR,
}
action = "creating"
// subnet will be delegated to ACI on both create and update scenarios
cgDelegation := &aznetworkv2.Delegation{
Name: &delegationName,
Properties: &aznetworkv2.ServiceDelegationPropertiesFormat{
ServiceName: &serviceName,
Actions: []*string{&subnetAction},
},
}
subnet.Properties.Delegations = append(subnet.Properties.Delegations, cgDelegation)

logger.Debugf("%s subnet %s", action, *subnet.Name)

Expand Down
4 changes: 2 additions & 2 deletions pkg/network/aci_network_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ func TestFormDNSNameserversFitsLimits(t *testing.T) {
}
}

func TestShouldCreateSubnet(t *testing.T) {
func TestShouldCreateOrUpdateSubnet(t *testing.T) {
subnetName := "fakeSubnet"
fakeAddPrefix := "10.00.0/16"
providerSubnetCIDR := "10.00.0/17"
Expand Down Expand Up @@ -260,7 +260,7 @@ func TestShouldCreateSubnet(t *testing.T) {
pn.SubnetCIDR = tc.providerSubnetCIDR
currentSubnet.Properties = &tc.subnetProperties

result, err := pn.shouldCreateSubnet(currentSubnet)
result, err := pn.shouldCreateOrUpdateSubnet(currentSubnet)

if tc.expectedError != nil {
assert.Equal(t, err.Error(), tc.expectedError.Error(), "Error messages should match")
Expand Down
Loading