Skip to content

Commit

Permalink
Review comments.
Browse files Browse the repository at this point in the history
  • Loading branch information
Gerrit91 committed Oct 28, 2024
1 parent c3a9ae7 commit e941970
Show file tree
Hide file tree
Showing 8 changed files with 18 additions and 17 deletions.
5 changes: 3 additions & 2 deletions metal/cloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"github.com/metal-stack/metal-ccm/pkg/controllers/housekeeping"
"github.com/metal-stack/metal-ccm/pkg/controllers/instances"
"github.com/metal-stack/metal-ccm/pkg/controllers/loadbalancer"
"github.com/metal-stack/metal-ccm/pkg/controllers/loadbalancer/config"
"github.com/metal-stack/metal-ccm/pkg/controllers/zones"
"github.com/metal-stack/metal-ccm/pkg/resources/constants"
"github.com/metal-stack/metal-ccm/pkg/resources/metal"
Expand Down Expand Up @@ -47,7 +48,7 @@ func NewCloud(_ io.Reader) (cloudprovider.Interface, error) {
partitionID := os.Getenv(constants.MetalPartitionIDEnvVar)
clusterID := os.Getenv(constants.MetalClusterIDEnvVar)
defaultExternalNetworkID := os.Getenv(constants.MetalDefaultExternalNetworkEnvVar)
loadbalancerType := os.Getenv(constants.Loadbalancer)
loadbalancerType := config.LoadBalancerType(os.Getenv(constants.Loadbalancer))

var (
additionalNetworksString = os.Getenv(constants.MetalAdditionalNetworks)
Expand Down Expand Up @@ -80,7 +81,7 @@ func NewCloud(_ io.Reader) (cloudprovider.Interface, error) {
return nil, fmt.Errorf("environment variable %q or %q is required", constants.MetalAuthTokenEnvVar, constants.MetalAuthHMACEnvVar)
}

if !slices.Contains([]string{"cilium", "metallb", ""}, loadbalancerType) {
if !slices.Contains([]config.LoadBalancerType{config.LoadBalancerTypeCilium, config.LoadBalancerTypeMetalLB, ""}, loadbalancerType) {
klog.Fatalf("only cilium or metallb load balancer types are supported")
}

Expand Down
9 changes: 1 addition & 8 deletions pkg/controllers/loadbalancer/config/addresspool.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,7 @@ func (pool *addressPool) appendIP(ip *models.V1IPResponse) error {
return err
}

var cidr string
if parsed.Is4() {
cidr = parsed.String() + "/32"
} else if parsed.Is6() {
cidr = parsed.String() + "/128"
} else {
return fmt.Errorf("unknown addressfamily of ip: %s", parsed.String())
}
cidr := fmt.Sprintf("%s/%d", parsed.String(), parsed.BitLen())

if pool.containsCIDR(cidr) {
return nil
Expand Down
4 changes: 4 additions & 0 deletions pkg/controllers/loadbalancer/config/cilium_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ import (
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
)

const (
LoadBalancerTypeCilium LoadBalancerType = "cilium"
)

type ciliumConfig struct {
base *baseConfig
client client.Client
Expand Down
2 changes: 1 addition & 1 deletion pkg/controllers/loadbalancer/config/cilium_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ func TestCiliumConfig(t *testing.T) {
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
cfg, err := New("cilium", tt.ips, tt.nws, tt.nodes, nil, nil)
cfg, err := New(LoadBalancerTypeCilium, tt.ips, tt.nws, tt.nodes, nil, nil)
if diff := cmp.Diff(err, tt.wantErr); diff != "" {
t.Errorf("error = %v", diff)
return
Expand Down
7 changes: 4 additions & 3 deletions pkg/controllers/loadbalancer/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"
)

type LoadBalancerType string
type LoadBalancerConfig interface {
WriteCRs(ctx context.Context) error
}
Expand All @@ -24,16 +25,16 @@ type baseConfig struct {
AddressPools addressPools `json:"address-pools,omitempty" yaml:"address-pools,omitempty"`
}

func New(loadBalancerType string, ips []*models.V1IPResponse, nws sets.Set[string], nodes []v1.Node, c client.Client, k8sClientSet clientset.Interface) (LoadBalancerConfig, error) {
func New(loadBalancerType LoadBalancerType, ips []*models.V1IPResponse, nws sets.Set[string], nodes []v1.Node, c client.Client, k8sClientSet clientset.Interface) (LoadBalancerConfig, error) {
bc, err := newBaseConfig(ips, nws, nodes)
if err != nil {
return nil, err
}

switch loadBalancerType {
case "metallb":
case LoadBalancerTypeMetalLB:
return newMetalLBConfig(bc, c), nil
case "cilium":
case LoadBalancerTypeCilium:
return newCiliumConfig(bc, c, k8sClientSet), nil
default:
return newMetalLBConfig(bc, c), nil
Expand Down
2 changes: 2 additions & 0 deletions pkg/controllers/loadbalancer/config/metallb_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ import (
)

const (
LoadBalancerTypeMetalLB LoadBalancerType = "metallb"

metallbNamespace = "metallb-system"
)

Expand Down
2 changes: 1 addition & 1 deletion pkg/controllers/loadbalancer/config/metallb_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ func TestMetalLBConfig(t *testing.T) {
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
cfg, err := New("metallb", tt.ips, tt.nws, tt.nodes, nil, nil)
cfg, err := New(LoadBalancerTypeMetalLB, tt.ips, tt.nws, tt.nodes, nil, nil)
if diff := cmp.Diff(err, tt.wantErr); diff != "" {
t.Errorf("error = %v", diff)
return
Expand Down
4 changes: 2 additions & 2 deletions pkg/controllers/loadbalancer/loadbalancer.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,11 @@ type LoadBalancerController struct {
configWriteMutex *sync.Mutex
ipAllocateMutex *sync.Mutex
ipUpdateMutex *sync.Mutex
loadBalancerType string
loadBalancerType config.LoadBalancerType
}

// New returns a new load balancer controller that satisfies the kubernetes cloud provider load balancer interface
func New(partitionID, projectID, clusterID, defaultExternalNetworkID string, additionalNetworks []string, loadBalancerType string) *LoadBalancerController {
func New(partitionID, projectID, clusterID, defaultExternalNetworkID string, additionalNetworks []string, loadBalancerType config.LoadBalancerType) *LoadBalancerController {
return &LoadBalancerController{
partitionID: partitionID,
projectID: projectID,
Expand Down

0 comments on commit e941970

Please sign in to comment.