Skip to content
This repository has been archived by the owner on Jul 16, 2024. It is now read-only.

IPVS Fullstate Support for IPv6 #443

Merged
merged 1 commit into from
Feb 15, 2023

Conversation

aroradaman
Copy link
Contributor

In continuation to #417.

Passes e2e for ipv4, ipv6 and dual on Ubuntu 22.04.1 LTS.

@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 29, 2023
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jan 29, 2023
@aroradaman aroradaman marked this pull request as ready for review January 29, 2023 02:52
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 29, 2023
@k8s-ci-robot k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Jan 29, 2023
@jayunit100
Copy link
Contributor

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Feb 3, 2023
@jayunit100
Copy link
Contributor

/retest

func getNodeIPs(ipFamily v1.IPFamily) []string {
IPs := make([]string, 0)
for _, ip := range *NodeAddresses {
if ipFamily == v1.IPv4Protocol && netutils.IsIPv4String(ip) {
Copy link
Contributor

Choose a reason for hiding this comment

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

wait cant this just be if IPv4 or IPv6 then

IPs = append(...

Copy link
Contributor Author

@aroradaman aroradaman Feb 3, 2023

Choose a reason for hiding this comment

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

func getExternalIPss(service *localv1.Service, ipFamily v1.IPFamily) ([]string, error) {
	if service.IPs.ExternalIPs != nil {
		if ipFamily == v1.IPv4Protocol {
			return service.IPs.ExternalIPs.GetV4(), nil
		} else if ipFamily == v1.IPv6Protocol {
			return service.IPs.ExternalIPs.GetV6(), nil
		}else{
			return nil, errors.New(fmt.Sprint("ipfamily [%v] not supported", ipFamily))		
		}
	} else {
		return nil, errors.New("no ips found")
	}
}

@jayunit100 something like this ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

wait can't this just be IPv4 or IPv6 then

Entries in an IPSet can only belong to one IPFamily, so I loop service creation logic for both the ipFamilies.

// iterate over ipFamily
for _, ipFamily := range []v1.IPFamily{v1.IPv4Protocol, v1.IPv6Protocol} {
    / /iterate over NodeIPs
    for _, nodeIP := range getNodeIPs(ipFamily) {
        ....
    }
}

And then I pick the relevant IPSet accordingly

kubeNodePortTCPIPSet  = map[v1.IPFamily]string{
    v1.IPv4Protocol: "KUBE-NODE-PORT-TCP",
    v1.IPv6Protocol: "KUBE-6-NODE-PORT-TCP",
}

@aroradaman
Copy link
Contributor Author

/retest

@k8s-ci-robot
Copy link
Contributor

@daman1807: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

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.

@jayunit100
Copy link
Contributor

/approve
/lgtm
/retest
/test-this-please

@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 13, 2023
@aroradaman
Copy link
Contributor Author

@jayunit100 all the tests (Security / govulncheck) pass now, we can merge this.

@jayunit100
Copy link
Contributor

/lgtm
/approve

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

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: daman1807, jayunit100

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 merged commit b4837e5 into kubernetes-retired:master Feb 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants