Skip to content

Commit

Permalink
address PR comments
Browse files Browse the repository at this point in the history
  • Loading branch information
bschimke95 committed Jun 13, 2024
1 parent a1c04d3 commit bbc707b
Show file tree
Hide file tree
Showing 15 changed files with 168 additions and 47 deletions.
2 changes: 1 addition & 1 deletion src/k8s/pkg/k8sd/setup/containerd.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ func Containerd(snap snap.Snap, registries []types.ContainerdRegistry, extraArgs
}

// Apply extra arguments after the defaults, so they can override them.
updateArgs, deleteArgs := snaputil.ServiceArgsFromMap(extraArgs)
updateArgs, deleteArgs := utils.ServiceArgsFromMap(extraArgs)
if _, err := snaputil.UpdateServiceArguments(snap, "containerd", updateArgs, deleteArgs); err != nil {
return fmt.Errorf("failed to write arguments file: %w", err)
}
Expand Down
3 changes: 2 additions & 1 deletion src/k8s/pkg/k8sd/setup/k8s_apiserver_proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/canonical/k8s/pkg/proxy"
"github.com/canonical/k8s/pkg/snap"
snaputil "github.com/canonical/k8s/pkg/snap/util"
"github.com/canonical/k8s/pkg/utils"
)

// K8sAPIServerProxy prepares configuration for k8s-apiserver-proxy.
Expand All @@ -25,7 +26,7 @@ func K8sAPIServerProxy(snap snap.Snap, servers []string, extraArgs map[string]*s
}

// Apply extra arguments after the defaults, so they can override them.
updateArgs, deleteArgs := snaputil.ServiceArgsFromMap(extraArgs)
updateArgs, deleteArgs := utils.ServiceArgsFromMap(extraArgs)
if _, err := snaputil.UpdateServiceArguments(snap, "k8s-apiserver-proxy", updateArgs, deleteArgs); err != nil {
return fmt.Errorf("failed to write arguments file: %w", err)
}
Expand Down
3 changes: 2 additions & 1 deletion src/k8s/pkg/k8sd/setup/k8s_dqlite.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (

"github.com/canonical/k8s/pkg/snap"
snaputil "github.com/canonical/k8s/pkg/snap/util"
"github.com/canonical/k8s/pkg/utils"
"gopkg.in/yaml.v2"
)

Expand All @@ -33,7 +34,7 @@ func K8sDqlite(snap snap.Snap, address string, cluster []string, extraArgs map[s
}

// Apply extra arguments after the defaults, so they can override them.
updateArgs, deleteArgs := snaputil.ServiceArgsFromMap(extraArgs)
updateArgs, deleteArgs := utils.ServiceArgsFromMap(extraArgs)
if _, err := snaputil.UpdateServiceArguments(snap, "k8s-dqlite", updateArgs, deleteArgs); err != nil {
return fmt.Errorf("failed to write arguments file: %w", err)
}
Expand Down
3 changes: 2 additions & 1 deletion src/k8s/pkg/k8sd/setup/kube_apiserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/canonical/k8s/pkg/k8sd/types"
"github.com/canonical/k8s/pkg/snap"
snaputil "github.com/canonical/k8s/pkg/snap/util"
"github.com/canonical/k8s/pkg/utils"
)

type apiserverAuthTokenWebhookTemplateConfig struct {
Expand Down Expand Up @@ -107,7 +108,7 @@ func KubeAPIServer(snap snap.Snap, serviceCIDR string, authWebhookURL string, en
}

// Apply extra arguments after the defaults, so they can override them.
updateArgs, deleteArgs := snaputil.ServiceArgsFromMap(extraArgs)
updateArgs, deleteArgs := utils.ServiceArgsFromMap(extraArgs)
if _, err := snaputil.UpdateServiceArguments(snap, "kube-apiserver", updateArgs, deleteArgs); err != nil {
return fmt.Errorf("failed to write arguments file: %w", err)
}
Expand Down
3 changes: 2 additions & 1 deletion src/k8s/pkg/k8sd/setup/kube_controller_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (

"github.com/canonical/k8s/pkg/snap"
snaputil "github.com/canonical/k8s/pkg/snap/util"
"github.com/canonical/k8s/pkg/utils"
)

// KubeControllerManager configures kube-controller-manager on the local node.
Expand All @@ -31,7 +32,7 @@ func KubeControllerManager(snap snap.Snap, extraArgs map[string]*string) error {
return fmt.Errorf("failed to render arguments file: %w", err)
}
// Apply extra arguments after the defaults, so they can override them.
updateArgs, deleteArgs := snaputil.ServiceArgsFromMap(extraArgs)
updateArgs, deleteArgs := utils.ServiceArgsFromMap(extraArgs)
if _, err := snaputil.UpdateServiceArguments(snap, "kube-controller-manager", updateArgs, deleteArgs); err != nil {
return fmt.Errorf("failed to write arguments file: %w", err)
}
Expand Down
3 changes: 2 additions & 1 deletion src/k8s/pkg/k8sd/setup/kube_proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

"github.com/canonical/k8s/pkg/snap"
snaputil "github.com/canonical/k8s/pkg/snap/util"
"github.com/canonical/k8s/pkg/utils"
)

// KubeProxy configures kube-proxy on the local node.
Expand Down Expand Up @@ -35,7 +36,7 @@ func KubeProxy(ctx context.Context, snap snap.Snap, hostname string, podCIDR str
}

// Apply extra arguments after the defaults, so they can override them.
updateArgs, deleteArgs := snaputil.ServiceArgsFromMap(extraArgs)
updateArgs, deleteArgs := utils.ServiceArgsFromMap(extraArgs)
if _, err := snaputil.UpdateServiceArguments(snap, "kube-proxy", updateArgs, deleteArgs); err != nil {
return fmt.Errorf("failed to write arguments file: %w", err)
}
Expand Down
3 changes: 2 additions & 1 deletion src/k8s/pkg/k8sd/setup/kube_scheduler.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (

"github.com/canonical/k8s/pkg/snap"
snaputil "github.com/canonical/k8s/pkg/snap/util"
"github.com/canonical/k8s/pkg/utils"
)

// KubeScheduler configures kube-scheduler on the local node.
Expand All @@ -21,7 +22,7 @@ func KubeScheduler(snap snap.Snap, extraArgs map[string]*string) error {
return fmt.Errorf("failed to render arguments file: %w", err)
}
// Apply extra arguments after the defaults, so they can override them.
updateArgs, deleteArgs := snaputil.ServiceArgsFromMap(extraArgs)
updateArgs, deleteArgs := utils.ServiceArgsFromMap(extraArgs)
if _, err := snaputil.UpdateServiceArguments(snap, "kube-scheduler", updateArgs, deleteArgs); err != nil {
return fmt.Errorf("failed to write arguments file: %w", err)
}
Expand Down
3 changes: 2 additions & 1 deletion src/k8s/pkg/k8sd/setup/kubelet.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

"github.com/canonical/k8s/pkg/snap"
snaputil "github.com/canonical/k8s/pkg/snap/util"
"github.com/canonical/k8s/pkg/utils"
)

var kubeletTLSCipherSuites = []string{
Expand Down Expand Up @@ -76,7 +77,7 @@ func kubelet(snap snap.Snap, hostname string, nodeIP net.IP, clusterDNS string,
}

// Apply extra arguments after the defaults, so they can override them.
updateArgs, deleteArgs := snaputil.ServiceArgsFromMap(extraArgs)
updateArgs, deleteArgs := utils.ServiceArgsFromMap(extraArgs)
if _, err := snaputil.UpdateServiceArguments(snap, "kubelet", updateArgs, deleteArgs); err != nil {
return fmt.Errorf("failed to write arguments file: %w", err)
}
Expand Down
14 changes: 1 addition & 13 deletions src/k8s/pkg/k8sd/setup/util_extra_files.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,27 +19,15 @@ func ExtraNodeConfigFiles(snap snap.Snap, files map[string]string) error {
}

filePath := path.Join(snap.ServiceExtraConfigDir(), filename)
// Create or truncate the file
file, err := os.Create(filePath)
if err != nil {
return fmt.Errorf("failed to create file %s: %w", filePath, err)
}
defer file.Close()

// Write the content to the file
_, err = file.WriteString(content)
if err != nil {
if err := os.WriteFile(filePath, []byte(content), 0400); err != nil {
return fmt.Errorf("failed to write to file %s: %w", filePath, err)
}

// Set file owner to root
if err := os.Chown(filePath, snap.UID(), snap.GID()); err != nil {
return fmt.Errorf("failed to change owner of file %s: %w", filePath, err)
}

if err := os.Chmod(filePath, 0400); err != nil {
return fmt.Errorf("failed to change mode of file %s: %w", filePath, err)
}
}
return nil
}
18 changes: 18 additions & 0 deletions src/k8s/pkg/utils/services.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package utils

// ServiceArgsFromMap processes a map of string pointers and categorizes them into update and delete lists.
// - If the value pointer is nil, it adds the argument name to the delete list.
// - If the value pointer is not nil, it adds the argument and its value to the update map.
func ServiceArgsFromMap(args map[string]*string) (map[string]string, []string) {
updateArgs := make(map[string]string)
deleteArgs := make([]string, 0)

for arg, val := range args {
if val == nil {
deleteArgs = append(deleteArgs, arg)
} else {
updateArgs[arg] = *val
}
}
return updateArgs, deleteArgs
}
77 changes: 77 additions & 0 deletions src/k8s/pkg/utils/services_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
package utils

import (
"testing"

. "github.com/onsi/gomega"
)

func TestServiceArgsFromMap(t *testing.T) {
tests := []struct {
name string
input map[string]*string
expected struct {
updateArgs map[string]string
deleteArgs []string
}
}{
{
name: "NilValue",
input: map[string]*string{"arg1": nil},
expected: struct {
updateArgs map[string]string
deleteArgs []string
}{
updateArgs: map[string]string{},
deleteArgs: []string{"arg1"},
},
},
{
name: "EmptyString", // Should be threated as normal string
input: map[string]*string{"arg1": Pointer("")},
expected: struct {
updateArgs map[string]string
deleteArgs []string
}{
updateArgs: map[string]string{"arg1": ""},
deleteArgs: []string{},
},
},
{
name: "NonEmptyString",
input: map[string]*string{"arg1": Pointer("value1")},
expected: struct {
updateArgs map[string]string
deleteArgs []string
}{
updateArgs: map[string]string{"arg1": "value1"},
deleteArgs: []string{},
},
},
{
name: "MixedValues",
input: map[string]*string{
"arg1": Pointer("value1"),
"arg2": Pointer(""),
"arg3": nil,
},
expected: struct {
updateArgs map[string]string
deleteArgs []string
}{
updateArgs: map[string]string{"arg1": "value1", "arg2": ""},
deleteArgs: []string{"arg3"},
},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
g := NewWithT(t)

updateArgs, deleteArgs := ServiceArgsFromMap(tt.input)
g.Expect(updateArgs).To(Equal(tt.expected.updateArgs))
g.Expect(deleteArgs).To(Equal(tt.expected.deleteArgs))
})
}
}
16 changes: 0 additions & 16 deletions tests/integration/templates/bootstrap-session.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,3 @@ cluster-config:
enabled: true
metrics-server:
enabled: true
extra-node-config-files:
bootstrap-extra-file.yaml: extra-args-test-file-content
extra-node-kube-apiserver-args:
--request-timeout: 2m
extra-node-kube-controller-manager-args:
--leader-elect-retry-period: 3s
extra-node-kube-scheduler-args:
--authorization-webhook-cache-authorized-ttl: 11s
extra-node-kube-proxy-args:
--config-sync-period: 14m
extra-node-kubelet-args:
--authentication-token-webhook-cache-ttl: 3m
extra-node-containerd-args:
--log-level: debug
extra-node-k8s-dqlite-args:
--watch-storage-available-size-interval: 6s
32 changes: 32 additions & 0 deletions tests/integration/templates/bootstrap-smoke.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# Contains the bootstrap configuration for the smoke test.
cluster-config:
network:
enabled: true
dns:
enabled: true
ingress:
enabled: true
load-balancer:
enabled: true
local-storage:
enabled: true
gateway:
enabled: true
metrics-server:
enabled: true
extra-node-config-files:
bootstrap-extra-file.yaml: extra-args-test-file-content
extra-node-kube-apiserver-args:
--request-timeout: 2m
extra-node-kube-controller-manager-args:
--leader-elect-retry-period: 3s
extra-node-kube-scheduler-args:
--authorization-webhook-cache-authorized-ttl: 11s
extra-node-kube-proxy-args:
--config-sync-period: 14m
extra-node-kubelet-args:
--authentication-token-webhook-cache-ttl: 3m
extra-node-containerd-args:
--log-level: debug
extra-node-k8s-dqlite-args:
--watch-storage-available-size-interval: 6s
31 changes: 23 additions & 8 deletions tests/integration/tests/test_smoke.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,39 @@
# Copyright 2024 Canonical, Ltd.
#
import logging
from typing import List

from test_util import harness
import pytest
from test_util import config, harness, util

LOG = logging.getLogger(__name__)


def test_smoke(session_instance: harness.Instance):
@pytest.mark.node_count(1)
@pytest.mark.disable_k8s_bootstrapping()
def test_smoke(instances: List[harness.Instance]):
instance = instances[0]

bootstrap_smoke_config_path = "/home/ubuntu/bootstrap-smoke.yaml"
instance.send_file(
(config.MANIFESTS_DIR / "bootstrap-smoke.yaml").as_posix(),
bootstrap_smoke_config_path,
)

instance.exec(["k8s", "bootstrap", "--file", bootstrap_smoke_config_path])
util.wait_until_k8s_ready(instance, [instance])

# Verify the functionality of the k8s config command during the smoke test.
# It would be excessive to deploy a cluster solely for this purpose.
result = session_instance.exec(
result = instance.exec(
"k8s config --server 192.168.210.41".split(), capture_output=True
)
config = result.stdout.decode()
assert len(config) > 0
assert "server: https://192.168.210.41" in config
kubeconfig = result.stdout.decode()
assert len(kubeconfig) > 0
assert "server: https://192.168.210.41" in kubeconfig

# Verify extra node configs
content = session_instance.exec(
content = instance.exec(
["cat", "/var/snap/k8s/common/args/conf.d/bootstrap-extra-file.yaml"],
capture_output=True,
)
Expand All @@ -35,7 +50,7 @@ def test_smoke(session_instance: harness.Instance):
"containerd": "--log-level=debug",
"k8s-dqlite": "--watch-storage-available-size-interval=6s",
}.items():
args = session_instance.exec(
args = instance.exec(
["cat", f"/var/snap/k8s/common/args/{service}"], capture_output=True
)
assert value in args.stdout.decode()
4 changes: 2 additions & 2 deletions tests/integration/tests/test_util/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -223,8 +223,8 @@ def get_join_token(


# Join an existing cluster.
def join_cluster(instance: harness.Instance, join_token: str, *args: str):
instance.exec(["k8s", "join-cluster", join_token, *args])
def join_cluster(instance: harness.Instance, join_token: str):
instance.exec(["k8s", "join-cluster", join_token])


def get_default_cidr(instance: harness.Instance, instance_default_ip: str):
Expand Down

0 comments on commit bbc707b

Please sign in to comment.