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.

Sign up for GitHub

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

Jump to bottom

[WIP] WINC-897: React to kubelet arg changes #2483

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
+317 −3
Draft

[WIP] WINC-897: React to kubelet arg changes #2483

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
73 changes: 70 additions & 3 deletions controllers/configmap_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,11 @@ import (
"fmt"
"net"
"reflect"
"regexp"
"strings"

config "github.com/openshift/api/config/v1"
mcfgv1 "github.com/openshift/api/machineconfiguration/v1"
core "k8s.io/api/core/v1"
rbac "k8s.io/api/rbac/v1"
k8sapierrors "k8s.io/apimachinery/pkg/api/errors"
Expand Down Expand Up @@ -186,7 +188,19 @@ func (r *ConfigMapReconciler) Reconcile(ctx context.Context,
// At this point configMap will be set properly
switch req.NamespacedName.Name {
case servicescm.Name:
return ctrl.Result{}, r.reconcileServices(ctx, configMap)
// get the machineConfig

if nodename, ok := ctx.Value("nodename").(string); ok {
r.log.Info("change triggered by node__from reconcile", "node", nodename)
node := &core.Node{}
if err := r.client.Get(ctx, kubeTypes.NamespacedName{
Name: nodename},
node); err != nil {
return ctrl.Result{}, err
}
return ctrl.Result{}, r.reconcileServices(ctx, configMap, node)
}

case wiparser.InstanceConfigMap:
return ctrl.Result{}, r.reconcileNodes(ctx, configMap)
case certificates.ProxyCertsConfigMap:
Expand All @@ -200,7 +214,7 @@ func (r *ConfigMapReconciler) Reconcile(ctx context.Context,

// reconcileServices uses the data within the services ConfigMap to ensure WMCO-managed Windows services on
// Windows Nodes have the expected configuration and are in the expected state
func (r *ConfigMapReconciler) reconcileServices(ctx context.Context, windowsServices *core.ConfigMap) error {
func (r *ConfigMapReconciler) reconcileServices(ctx context.Context, windowsServices *core.ConfigMap, node *core.Node) error {
if err := r.removeOutdatedServicesConfigMaps(ctx); err != nil {
return err
}
Expand All @@ -216,10 +230,50 @@ func (r *ConfigMapReconciler) reconcileServices(ctx context.Context, windowsServ
kubeTypes.NamespacedName{Namespace: r.watchNamespace, Name: windowsServices.Name}, "Error", err.Error())
return nil
}
r.setValues(node, data)
// TODO: actually react to changes to the services ConfigMap
return nil
}

func (r *ConfigMapReconciler) setValues(node *core.Node, data *servicescm.Data) {
// machineconfig.Spec.Config
// unmarshal the json, and set the data to the machineconfig's value

// check the node.annotations and node.label

for i := range data.Services {
value := &data.Services[i]
if value.Name == "kubelet" {
re := regexp.MustCompile(`(--node-labels=[^\s]+)`)
match := re.FindStringSubmatch(value.Command)
if len(match) > 1 {
currentNodeLabels := match[1]

labelsString := strings.TrimPrefix(currentNodeLabels, "--node-labels=")
labelsArray := strings.Split(labelsString, ",")
labelsMap := make(map[string]string)

for _, label := range labelsArray {
parts := strings.SplitN(label, "=", 2)
if len(parts) == 2 {
labelsMap[parts[0]] = parts[1]
}
}
for key, value := range node.Labels {
labelsMap[key] = value
}
var updatedLabels []string
for k, v := range labelsMap {
updatedLabels = append(updatedLabels, fmt.Sprintf("%s=%s", k, v))
}
newNodeLabels := "--node-labels=" + strings.Join(updatedLabels, ",")

value.Command = strings.Replace(value.Command, currentNodeLabels, newNodeLabels, 1)
}
}
}
}

// removeOutdatedServicesConfigMaps deletes any outdated services ConfigMaps, if all nodes have moved past that version
func (r *ConfigMapReconciler) removeOutdatedServicesConfigMaps(ctx context.Context) error {
nodes := &core.NodeList{}
Expand Down Expand Up @@ -386,7 +440,18 @@ func (r *ConfigMapReconciler) mapToInstancesConfigMap(_ context.Context, _ clien
}

// mapToServicesConfigMap fulfills the MapFn type, while always returning a request to the windows-services ConfigMap
func (r *ConfigMapReconciler) mapToServicesConfigMap(_ context.Context, _ client.Object) []reconcile.Request {
func (r *ConfigMapReconciler) mapToServicesConfigMap(ctx context.Context, obj client.Object) []reconcile.Request {
switch resource := obj.(type) {
case *core.Node:
nodeName := resource.GetName()
ctx = context.WithValue(ctx, "nodename", nodeName)
case *mcfgv1.MachineConfig:
machineconfigname := resource.GetName()
ctx = context.WithValue(ctx, "machineconfigname", machineconfigname)
default:
return nil
}

return []reconcile.Request{{
NamespacedName: kubeTypes.NamespacedName{Namespace: r.watchNamespace, Name: servicescm.Name},
}}
Expand Down Expand Up @@ -414,6 +479,8 @@ func (r *ConfigMapReconciler) SetupWithManager(mgr ctrl.Manager) error {
builder.WithPredicates(outdatedWindowsNodePredicate(true))).
Watches(&core.Node{}, handler.EnqueueRequestsFromMapFunc(r.mapToServicesConfigMap),
builder.WithPredicates(windowsNodeVersionChangePredicate())).
Watches(&core.Node{}, handler.EnqueueRequestsFromMapFunc(r.mapToServicesConfigMap),
builder.WithPredicates(nodeLabelChangedPredicate())).
Complete(r)
}

Expand Down
227 changes: 227 additions & 0 deletions controllers/configmap_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package controllers

import (
"regexp"
"strings"
"testing"

"github.com/stretchr/testify/require"
Expand All @@ -13,6 +15,231 @@ import (
"github.com/openshift/windows-machine-config-operator/pkg/wiparser"
)

func TestReconcileWindowsServices(t *testing.T) {
watchNamespace := "test"
r := ConfigMapReconciler{instanceReconciler: instanceReconciler{watchNamespace: watchNamespace}}
var tests = []struct {
name string
configMapObj client.Object
expectedLabels string
node *core.Node
}{{
name: "addNewLabel",
configMapObj: &core.ConfigMap{
ObjectMeta: meta.ObjectMeta{
Name: wiparser.InstanceConfigMap,
Namespace: watchNamespace,
},
Data: map[string]string{
"services": `[
{
"name": "containerd",
"path": "C:/k/containerd/containerd.exe
--config C:/k/containerd/containerd_conf.toml
--log-file C:/var/log/containerd/containerd.log
--run-service
--log-level info",
"powershellPreScripts": [
{
"path": "C:/Temp/windows-defender-exclusion.ps1 -BinPath C:/k/containerd/containerd.exe"
}
],
"bootstrap": true,
"priority": 0
},
{
"name": "kubelet",
"path": "C:/k/kube-log-runner.exe -log-file=C:/var/log/kubelet/kubelet.log
C:/k/kubelet.exe
--config=C:/k/kubelet.conf
--bootstrap-kubeconfig=C:/k/bootstrap-kubeconfig
--kubeconfig=C:/k/kubeconfig
--cert-dir=c:/var/lib/kubelet/pki/
--windows-service
--node-labels=node.openshift.io/os_id=Windows
--windows-priorityclass=ABOVE_NORMAL_PRIORITY_CLASS
--v=2
--cloud-provider=external
--hostname-override=HOSTNAME_OVERRIDE
--node-ip=NODE_IP
--image-credential-provider-bin-dir=C:/k
--image-credential-provider-config=C:/k/credential-provider-config.yaml",
"powershellPreScripts": [
{
"variableName": "HOSTNAME_OVERRIDE",
"path": "Invoke-RestMethod -UseBasicParsing -Uri http://169.254.169.254/latest/meta-data/local-hostname"
},
{
"variableName": "NODE_IP",
"path": "(Get-NetRoute -DestinationPrefix '0.0.0.0/0' | Get-NetIpAddress -AddressFamily IPv4 -ifIndex {$_.ifIndex}[0]).IPAddress"
}
],
"dependencies": ["containerd"],
"bootstrap": true,
"priority": 1
},
{
"name": "windows_exporter",
"path": "C:/k/windows_exporter.exe --collectors.enabled cpu,cs,logical_disk,net,os,service,system,textfile,container,memory,cpu_info
--web.config.file C:/k/tls/windows-exporter-webconfig.yaml",
"bootstrap": false,
"priority": 2
},
{
"name": "hybrid-overlay-node",
"path": "C:/k/hybrid-overlay-node.exe --node NODE_NAME
--bootstrap-kubeconfig=C:/k/kubeconfig
--cert-dir=C:/k/cni/config
--cert-duration=24h
--windows-service
--logfile C:/var/log/hybrid-overlay/hybrid-overlay.log",
"nodeVariablesInCommand": [
{
"name": "NODE_NAME",
"nodeObjectJsonPath": "{.metadata.name}"
}
],
"dependencies": ["kubelet"],
"bootstrap": false,
"priority": 2
},
{
"name": "csi-proxy",
"path": "C:/k/csi-proxy.exe -log_file=C:/var/log/csi-proxy/csi-proxy.log
-logtostderr=false -windows-service --v=2",
"bootstrap": false,
"priority": 2
},
{
"name": "kube-proxy",
"path": "C:/k/kube-log-runner.exe -log-file=C:/var/log/kube-proxy/kube-proxy.log
C:/k/kube-proxy.exe
--windows-service
--proxy-mode=kernelspace
--feature-gates=WinOverlay=true,WinDSR=true
--hostname-override=NODE_NAME
--kubeconfig=C:/k/kubeconfig
--cluster-cidr=NODE_SUBNET
--network-name=OVNKubernetesHybridOverlayNetwork
--source-vip=ENDPOINT_IP
--enable-dsr=true
--v=2",
"nodeVariablesInCommand": [
{
"name": "NODE_NAME",
"nodeObjectJsonPath": "{.metadata.name}"
},
{
"name": "NODE_SUBNET",
"nodeObjectJsonPath": "{.metadata.annotations.k8s.ovn.org/hybrid-overlay-node-subnet}"
}
],
"powershellPreScripts": [
{
"variableName": "ENDPOINT_IP",
"path": "C:/Temp/network-conf.ps1"
}
],
"dependencies": ["hybrid-overlay-node"],
"bootstrap": false,
"priority": 3
}
]`,
"watchedEnvironmentVars": `["HTTP_PROXY", "HTTPS_PROXY", "NO_PROXY"]`,
"files": `[]`,
},
},
expectedLabels: "--node-labels=node.openshift.io/os_id=Windows,example-label-key=example-label-value",
node: &core.Node{
ObjectMeta: meta.ObjectMeta{
Name: "example-node",
Annotations: map[string]string{
"example-annotation-key": "example-annotation-value",
},
Labels: map[string]string{
"example-label-key": "example-label-value",
},
},
},
},
{
name: "updateExistingLabel",
configMapObj: &core.ConfigMap{
ObjectMeta: meta.ObjectMeta{
Name: wiparser.InstanceConfigMap,
Namespace: watchNamespace,
},
Data: map[string]string{
"services": `[{
"name": "kubelet",
"path": "--node-labels=node.openshift.io/os_id=Windows"}]`,

"watchedEnvironmentVars": `["HTTP_PROXY", "HTTPS_PROXY", "NO_PROXY"]`,
"files": `[]`,
},
},
expectedLabels: "--node-labels=node.openshift.io/os_id=NotWindows,example-label-key=example-label-value",
node: &core.Node{
ObjectMeta: meta.ObjectMeta{
Name: "example-node",
Labels: map[string]string{
"example-label-key": "example-label-value",
"node.openshift.io/os_id": "NotWindows",
},
},
},
},
{
name: "updateExistingLabel",
configMapObj: &core.ConfigMap{
ObjectMeta: meta.ObjectMeta{
Name: wiparser.InstanceConfigMap,
Namespace: watchNamespace,
},
Data: map[string]string{
"services": `[{
"name": "kubelet",
"path": "--node-labels=node.openshift.io/os_id=Windows"}]`,

"watchedEnvironmentVars": `["HTTP_PROXY", "HTTPS_PROXY", "NO_PROXY"]`,
"files": `[]`,
},
},
expectedLabels: "--node-labels=node.openshift.io/os_id=NotWindows,example-label-key=example-label-value",
node: &core.Node{
ObjectMeta: meta.ObjectMeta{
Name: "example-node",
Labels: map[string]string{
"example-label-key": "example-label-value",
"node.openshift.io/os_id": "NotWindows",
},
},
},
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
// this strips the newlines and tabs to allow for it to be parsed properly
// I am leaving the newlines and tabs in for test readability
replacer := strings.NewReplacer("\n", "", "\t", "")
test.configMapObj.(*core.ConfigMap).Data["services"] = replacer.Replace(test.configMapObj.(*core.ConfigMap).Data["services"])

data, err := servicescm.Parse(test.configMapObj.(*core.ConfigMap).Data)
if err != nil {
t.Errorf("invalid data %v", err)
}
r.setValues(test.node, data)
re := regexp.MustCompile(`(--node-labels=[^\s]+)`)
for _, value := range data.Services {
if value.Name == "kubelet" {
match := re.FindStringSubmatch(value.Command)
require.Equal(t, match[1], test.expectedLabels)
}
}
})
}
}

func TestIsValidConfigMap(t *testing.T) {
watchNamespace := "test"
r := ConfigMapReconciler{instanceReconciler: instanceReconciler{watchNamespace: watchNamespace}}
Expand Down
20 changes: 20 additions & 0 deletions controllers/controllers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"
"net"
"reflect"
"sync"

"github.com/go-logr/logr"
Expand Down Expand Up @@ -201,6 +202,25 @@ func windowsNodeVersionChangePredicate() predicate.Funcs {
}
}

func nodeLabelChangedPredicate() predicate.Funcs {
return predicate.Funcs{
CreateFunc: func(e event.CreateEvent) bool {
return false
},
UpdateFunc: func(e event.UpdateEvent) bool {
// check if node object metadata.labels = e.objectold vs e.objectnew
return !reflect.DeepEqual(e.ObjectOld.GetLabels(), e.ObjectNew.GetLabels())
},
GenericFunc: func(e event.GenericEvent) bool {
return false

},
DeleteFunc: func(e event.DeleteEvent) bool {
return false
},
}
}

// outdatedWindowsNodePredicate returns a predicate which filters out all node objects that are not up-to-date Windows
// nodes. Up-to-date refers to the version annotation and public key hash annotations.
// If BYOH is true, only BYOH nodes will be allowed through, else no BYOH nodes will be allowed.
Expand Down