Skip to content

Commit

Permalink
Allow empty default network. (#87)
Browse files Browse the repository at this point in the history
  • Loading branch information
Gerrit91 authored Jan 29, 2024
1 parent 9a4480f commit f60d7e7
Show file tree
Hide file tree
Showing 7 changed files with 49 additions and 55 deletions.
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

0 comments on commit f60d7e7

Please sign in to comment.