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

Implements a secondary interface just for services #266

Merged
merged 2 commits into from
Sep 4, 2021

Conversation

thebsdbox
Copy link
Collaborator

This allows an end user to specify a secondary interface that is used purely for services.

--interface eth0
--serviceInterface eth1

Or within the environment variables!

    name: vip_serviceinterface
    value: eth1

@@ -141,6 +141,6 @@ var kubeVipSampleManifest = &cobra.Command{
}

b, _ := yaml.Marshal(p)
fmt.Printf(string(b))
fmt.Print(string(b))
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it just for testing? It's better to delete it.

@@ -150,7 +152,7 @@ func autoGenLocalPeer() (*kubevip.RaftPeer, error) {
}
}
if a == "" {
return nil, fmt.Errorf("Unable to find local address")
return nil, fmt.Errorf("nable to find local address")
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it should be unable.

@@ -24,6 +24,9 @@ const (
//vipInterface - defines the interface that the vip should bind too
vipInterface = "vip_interface"

//vipInterface - defines the interface that the vip should bind too
Copy link
Contributor

Choose a reason for hiding this comment

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

wrong annotation

@@ -24,6 +24,9 @@ const (
//vipInterface - defines the interface that the vip should bind too
vipInterface = "vip_interface"

//vipInterface - defines the interface that the vip should bind too
vipServicesInterface = "vip_servicesinterface"
Copy link
Contributor

Choose a reason for hiding this comment

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

From my perspect, services_vip_interface would be better than vip_servicesinterface. What do you think?

@@ -63,6 +63,9 @@ type Config struct {
// Interface is the network interface to bind to (default: First Adapter)
Interface string `yaml:"interface,omitempty"`

// Interface is the network interface to bind to (default: First Adapter)
Copy link
Contributor

Choose a reason for hiding this comment

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

wrong annotation.

@@ -90,18 +90,26 @@ func (sm *Manager) syncServices(service *v1.Service, wg *sync.WaitGroup) error {

}

// Detect if we're using a specific interface for services
var serciceInterface string
Copy link
Contributor

Choose a reason for hiding this comment

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

spelling mistake

// Generate new Virtual IP configuration
newVip := kubevip.Config{
VIP: newServiceAddress, //TODO support more than one vip?
Interface: sm.config.Interface,
Interface: serciceInterface,
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

@thebsdbox
Copy link
Collaborator Author

Thanks for the feedback, all issues addressed!

@yaocw2020
Copy link
Contributor

lgtm

@thebsdbox thebsdbox merged commit ab19d4a into kube-vip:main Sep 4, 2021
@thebsdbox thebsdbox deleted the nic_seperation branch October 13, 2021 10:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants