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

Allow empty default network. #87

Merged
merged 4 commits into from
Jan 29, 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
9 changes: 5 additions & 4 deletions .github/workflows/docker.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,20 @@ jobs:

steps:
- name: Log in to the container registry
uses: docker/login-action@v2
uses: docker/login-action@v3
with:
registry: ${{ env.REGISTRY }}
username: ${{ secrets.DOCKER_REGISTRY_USER }}
password: ${{ secrets.DOCKER_REGISTRY_TOKEN }}

- name: Checkout
uses: actions/checkout@v3
uses: actions/checkout@v4

- name: Set up Go 1.21
uses: actions/setup-go@v4
uses: actions/setup-go@v5
with:
go-version: '1.21.x'
cache: false

- name: Lint
uses: golangci/golangci-lint-action@v3
Expand All @@ -48,7 +49,7 @@ jobs:
[ "${GITHUB_EVENT_NAME}" == 'push' ] && echo "tag=latest" >> $GITHUB_ENV || true

- name: Build and push image
uses: docker/build-push-action@v4
uses: docker/build-push-action@v5
with:
context: .
push: true
Expand Down
4 changes: 0 additions & 4 deletions metal/cloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,6 @@ func NewCloud(_ io.Reader) (cloudprovider.Interface, error) {
return nil, fmt.Errorf("environment variable %q is required", constants.MetalClusterIDEnvVar)
}

if defaultExternalNetworkID == "" {
return nil, fmt.Errorf("environment variable %q is required", constants.MetalDefaultExternalNetworkEnvVar)
}

if url == "" {
return nil, fmt.Errorf("environment variable %q is required", constants.MetalAPIUrlEnvVar)
}
Expand Down
5 changes: 5 additions & 0 deletions pkg/controllers/instances/instances.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,11 @@ func nodeAddresses(machine *models.V1MachineResponse, defaultExternalNetwork str
continue
}

if defaultExternalNetwork == "" {
// empty default external network assumes isolated-cluster with forbidden access, so these nodes don't have an external IP
continue
}

if *nw.Networkid == defaultExternalNetwork {
for _, ip := range nw.Ips {
addresses = append(addresses, v1.NodeAddress{Type: v1.NodeExternalIP, Address: ip})
Expand Down
10 changes: 7 additions & 3 deletions pkg/controllers/loadbalancer/addresspool.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
package loadbalancer

import "fmt"
import (
"fmt"

"github.com/metal-stack/metal-lib/pkg/pointer"
)

const (
bgpProtocol = "bgp"
Expand All @@ -13,11 +17,11 @@ type AddressPool struct {
CIDRs []string `json:"addresses,omitempty" yaml:"addresses,omitempty"` // It is assumed that only /32 addresses are used.
}

func NewBGPAddressPool(name string, autoAssign bool) *AddressPool {
func NewBGPAddressPool(name string) *AddressPool {
return &AddressPool{
Name: name,
Protocol: bgpProtocol,
AutoAssign: &autoAssign,
AutoAssign: pointer.Pointer(false),
}
}

Expand Down
12 changes: 6 additions & 6 deletions pkg/controllers/loadbalancer/loadbalancer.go
Original file line number Diff line number Diff line change
Expand Up @@ -309,15 +309,15 @@ func (l *LoadBalancerController) acquireIP(ctx context.Context, service *v1.Serv
annotations := service.GetAnnotations()
addressPool, ok := annotations[constants.MetalLBSpecificAddressPool]
if !ok {
return l.acquireIPFromDefaultExternalNetwork(ctx, service)
if l.defaultExternalNetworkID == "" {
return "", fmt.Errorf(`no default network for ip acquisition specified, acquire an ip for your cluster's project and specify it directly in "spec.loadBalancerIP"`)
}

return l.acquireIPFromSpecificNetwork(ctx, service, l.defaultExternalNetworkID)
}
return l.acquireIPFromSpecificNetwork(ctx, service, addressPool)
}

func (l *LoadBalancerController) acquireIPFromDefaultExternalNetwork(ctx context.Context, service *v1.Service) (string, error) {
return l.acquireIPFromSpecificNetwork(ctx, service, l.defaultExternalNetworkID)
}

func (l *LoadBalancerController) acquireIPFromSpecificNetwork(ctx context.Context, service *v1.Service, addressPoolName string) (string, error) {
nwID := strings.TrimSuffix(addressPoolName, "-"+models.V1IPBaseTypeEphemeral)
nwID = strings.TrimSuffix(nwID, "-"+models.V1IPBaseTypeEphemeral)
Expand All @@ -337,7 +337,7 @@ func (l *LoadBalancerController) updateLoadBalancerConfig(ctx context.Context, n
return fmt.Errorf("could not find ips of this project's cluster: %w", err)
}

config := newMetalLBConfig(l.defaultExternalNetworkID)
config := newMetalLBConfig()
err = config.CalculateConfig(ips, l.additionalNetworks, nodes)
if err != nil {
return err
Expand Down
19 changes: 7 additions & 12 deletions pkg/controllers/loadbalancer/metallb.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,12 @@ const (

// MetalLBConfig is a struct containing a config for metallb
type MetalLBConfig struct {
Peers []*Peer `json:"peers,omitempty" yaml:"peers,omitempty"`
AddressPools []*AddressPool `json:"address-pools,omitempty" yaml:"address-pools,omitempty"`
defaultNetworkID string
Peers []*Peer `json:"peers,omitempty" yaml:"peers,omitempty"`
AddressPools []*AddressPool `json:"address-pools,omitempty" yaml:"address-pools,omitempty"`
}

func newMetalLBConfig(defaultNetworkID string) *MetalLBConfig {
return &MetalLBConfig{
defaultNetworkID: defaultNetworkID,
}
func newMetalLBConfig() *MetalLBConfig {
return &MetalLBConfig{}
}

// CalculateConfig computes the metallb config from given parameter input.
Expand Down Expand Up @@ -101,14 +98,14 @@ func (cfg *MetalLBConfig) Write(ctx context.Context, client clientset.Interface)

// getOrCreateAddressPool returns the address pool of the given network.
// It will be created if it does not exist yet.
func (cfg *MetalLBConfig) getOrCreateAddressPool(poolName string, autoAssign bool) *AddressPool {
func (cfg *MetalLBConfig) getOrCreateAddressPool(poolName string) *AddressPool {
for _, pool := range cfg.AddressPools {
if pool.Name == poolName {
return pool
}
}

pool := NewBGPAddressPool(poolName, autoAssign)
pool := NewBGPAddressPool(poolName)
cfg.AddressPools = append(cfg.AddressPools, pool)

return pool
Expand All @@ -118,13 +115,11 @@ func (cfg *MetalLBConfig) getOrCreateAddressPool(poolName string, autoAssign boo
func (cfg *MetalLBConfig) addIPToPool(network string, ip models.V1IPResponse) {
t := ip.Type
poolType := models.V1IPBaseTypeEphemeral
autoAssign := network == cfg.defaultNetworkID
if t != nil && *t == models.V1IPBaseTypeStatic {
poolType = models.V1IPBaseTypeStatic
autoAssign = false
}
poolName := fmt.Sprintf("%s-%s", network, poolType)
pool := cfg.getOrCreateAddressPool(poolName, autoAssign)
pool := cfg.getOrCreateAddressPool(poolName)
pool.appendIP(*ip.Ipaddress)
}

Expand Down
45 changes: 19 additions & 26 deletions pkg/controllers/loadbalancer/metallb_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,18 +25,16 @@ var (

func TestMetalLBConfig_CalculateConfig(t *testing.T) {
tests := []struct {
name string
defaultNetworkID string
nws sets.Set[string]
ips []*models.V1IPResponse
nodes []v1.Node
wantErr error
want map[string]interface{}
name string
nws sets.Set[string]
ips []*models.V1IPResponse
nodes []v1.Node
wantErr error
want map[string]interface{}
}{
{
name: "one ip acquired, no nodes",
defaultNetworkID: "internet",
nws: testNetworks,
name: "one ip acquired, no nodes",
nws: testNetworks,
ips: []*models.V1IPResponse{
{
Ipaddress: pointer.String("84.1.1.1"),
Expand All @@ -57,17 +55,16 @@ func TestMetalLBConfig_CalculateConfig(t *testing.T) {
"addresses": []string{
"84.1.1.1/32",
},
"auto-assign": true,
"auto-assign": false,
"name": "internet-ephemeral",
"protocol": "bgp",
},
},
},
},
{
name: "two ips acquired, no nodes",
defaultNetworkID: "internet",
nws: testNetworks,
name: "two ips acquired, no nodes",
nws: testNetworks,
ips: []*models.V1IPResponse{
{
Ipaddress: pointer.String("84.1.1.1"),
Expand Down Expand Up @@ -99,17 +96,16 @@ func TestMetalLBConfig_CalculateConfig(t *testing.T) {
"84.1.1.1/32",
"84.1.1.2/32",
},
"auto-assign": true,
"auto-assign": false,
"name": "internet-ephemeral",
"protocol": "bgp",
},
},
},
},
{
name: "two ips acquired, one static ip, no nodes",
defaultNetworkID: "internet",
nws: testNetworks,
name: "two ips acquired, one static ip, no nodes",
nws: testNetworks,
ips: []*models.V1IPResponse{
{
Ipaddress: pointer.String("84.1.1.1"),
Expand Down Expand Up @@ -151,7 +147,7 @@ func TestMetalLBConfig_CalculateConfig(t *testing.T) {
"84.1.1.1/32",
"84.1.1.2/32",
},
"auto-assign": true,
"auto-assign": false,
"name": "internet-ephemeral",
"protocol": "bgp",
},
Expand All @@ -168,9 +164,8 @@ func TestMetalLBConfig_CalculateConfig(t *testing.T) {
},

{
name: "connected to internet,storage,dmz and mpls, two ips acquired, one static ip, no nodes",
defaultNetworkID: "internet",
nws: testNetworks,
name: "connected to internet,storage,dmz and mpls, two ips acquired, one static ip, no nodes",
nws: testNetworks,
ips: []*models.V1IPResponse{
{
Ipaddress: pointer.String("84.1.1.1"),
Expand Down Expand Up @@ -252,7 +247,7 @@ func TestMetalLBConfig_CalculateConfig(t *testing.T) {
"84.1.1.1/32",
"84.1.1.2/32",
},
"auto-assign": true,
"auto-assign": false,
"name": "internet-ephemeral",
"protocol": "bgp",
},
Expand Down Expand Up @@ -303,9 +298,7 @@ func TestMetalLBConfig_CalculateConfig(t *testing.T) {
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
cfg := &MetalLBConfig{
defaultNetworkID: tt.defaultNetworkID,
}
cfg := &MetalLBConfig{}

err := cfg.CalculateConfig(tt.ips, tt.nws, tt.nodes)
if diff := cmp.Diff(err, tt.wantErr); diff != "" {
Expand Down