kubeovn / kube-ovn

A Bridge between SDN and Cloud Native (Project under CNCF)
https://kubeovn.github.io/docs/stable/en/
Apache License 2.0
1.97k stars 450 forks source link

[BUG] Recreating VPC/Subnet after their deletion yields errors #4530

Closed SkalaNetworks closed 1 month ago

SkalaNetworks commented 1 month ago

Kube-OVN Version

v1.13.0

Kubernetes Version

v1.30.4+k0s

K0s

Operation-system/Kernel Version

"Debian GNU/Linux 12 (bookworm)" 6.1.0-18-amd64

Description

After deleting VPCs and their associated subnets, recreating them identically yields database errors.

Steps To Reproduce

  1. Create namespace "ns1" and "ns2"
  2. Apply the following configuration "kubectl apply -f test.yaml"
    
    kind: Vpc
    apiVersion: kubeovn.io/v1
    metadata:
    name: test-vpc-1
    spec:
    namespaces:
    - ns1
    ---
    kind: Vpc
    apiVersion: kubeovn.io/v1
    metadata:
    name: test-vpc-2
    spec:
    namespaces:
    - ns2
    ---

kind: Subnet apiVersion: kubeovn.io/v1 metadata: name: net1 spec: vpc: test-vpc-1 cidrBlock: 10.0.1.0/24,fd00:10:10::/64 namespaces:


apiVersion: v1 kind: Pod metadata: namespace: ns1 name: vpc1-pod spec: containers:

Current Behavior

VPCs/Subnets fail to work with the same name after they've been deleted and recreated

Expected Behavior

Can recreate them just fine

dosubot[bot] commented 1 month ago

To resolve the database errors when recreating VPCs and Subnets with the same names after deletion in Kube-OVN v1.13.0 on Kubernetes v1.30.4+k0s, you need to ensure that VPCs and Subnets do not share the same name. The provided code snippet from pkg/webhook/subnet.go shows that there is a validation check in place to prevent VPCs and Subnets from having the same name.

Here is the relevant part of the code:

vpcList := &ovnv1.VpcList{}
if err := v.cache.List(ctx, vpcList); err != nil {
    return ctrlwebhook.Errored(http.StatusBadRequest, err)
}
for _, item := range vpcList.Items {
    if item.Name == o.Name {
        err := errors.New("vpc and subnet cannot have the same name")
        return ctrlwebhook.Errored(http.StatusBadRequest, err)
    }
}

This code checks if any existing VPC has the same name as the Subnet being created and returns an error if it does. To resolve the issue, ensure that when you recreate VPCs and Subnets, they have unique names that do not conflict with each other.

Additionally, the pkg/controller/subnet.go file contains logic to handle various aspects of subnet creation and updates, including validation and formatting. For example, the validateVpcBySubnet function ensures that the VPC associated with a subnet is in a valid state and does not have overlapping namespaces with other VPCs. This further emphasizes the importance of maintaining unique and non-conflicting names and configurations for VPCs and Subnets.

Here is a snippet from pkg/controller/subnet.go:

func (c *Controller) validateVpcBySubnet(subnet *kubeovnv1.Subnet) (*kubeovnv1.Vpc, error) {
    vpc, err := c.vpcsLister.Get(subnet.Spec.Vpc)
    if err != nil {
        klog.Errorf("failed to get subnet's vpc '%s', %v", subnet.Spec.Vpc, err)
        return vpc, err
    }

    if !vpc.Status.Standby {
        err = fmt.Errorf("the vpc '%s' not standby yet", vpc.Name)
        klog.Error(err)
        return vpc, err
    }

    if !vpc.Status.Default {
        for _, ns := range subnet.Spec.Namespaces {
            if !slices.Contains(vpc.Spec.Namespaces, ns) {
                err = fmt.Errorf("namespace '%s' is out of range to custom vpc '%s'", ns, vpc.Name)
                klog.Error(err)
                return vpc, err
            }
        }
    } else {
        vpcs, err := c.vpcsLister.List(labels.Everything())
        if err != nil {
            klog.Errorf("failed to list vpc, %v", err)
            return vpc, err
        }
        for _, vpc := range vpcs {
            if (subnet.Annotations[util.VpcLastName] == "" && subnet.Spec.Vpc != vpc.Name ||
                subnet.Annotations[util.VpcLastName] != "" && subnet.Annotations[util.VpcLastName] != vpc.Name) &&
                !vpc.Status.Default && util.IsStringsOverlap(vpc.Spec.Namespaces, subnet.Spec.Namespaces) {
                err = fmt.Errorf("namespaces %v are overlap with vpc '%s'", subnet.Spec.Namespaces, vpc.Name)
                klog.Error(err)
                return vpc, err
            }
        }
    }
    return vpc, nil
}

By ensuring unique names and validating configurations, you can avoid database errors when recreating VPCs and Subnets [1][2][3].

To continue talking to Dosu, mention @dosu.

SkalaNetworks commented 1 month ago

So apparently VPC and subnets shouldn't have the same name, this is a very weird requirement, is this warranted?

dosubot[bot] commented 1 month ago

Yes, it is warranted that VPCs and subnets should not have the same name in Kube-OVN. This is enforced in the VpcCreateHook function, where it checks if any existing subnet has the same name as the VPC being created and returns an error if they do.

for _, item := range subnetList.Items {
    if item.Name == vpc.Name {
        err := errors.New("vpc and subnet cannot have the same name")
        return ctrlwebhook.Errored(http.StatusBadRequest, err)
    }
}

This requirement helps to avoid conflicts and ensures the integrity of the system [1].

To continue talking to Dosu, mention @dosu.

oilbeater commented 1 month ago

@bobz965 do you remember why we have this restriction?

bobz965 commented 1 month ago

VPC and subnets shouldn't have the same name

Maybe, LSP and LRP should have the same name when vpc and subnet has the same name. but LSP and LRP are both the Port resources?

image