Skip to content

Commit

Permalink
install: Move the logic to unschedule aws-node to preinstall
Browse files Browse the repository at this point in the history
- Move the logic to unschedule aws-node pods from Install to preinstall
  so that it can be shared between classic and helm mode.
- Look for cni.chainingMode Helm value instead of generating configmap
  and looking for cni-chaining-mode field.

It is alarming to see cilium-cli mutating objects that it does not own,
but I guess that's a separate discussion altogether...

Signed-off-by: Michi Mutsuzaki <[email protected]>
  • Loading branch information
michi-covalent committed May 17, 2023
1 parent 371a966 commit 493031b
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 17 deletions.
48 changes: 31 additions & 17 deletions install/install.go
Original file line number Diff line number Diff line change
Expand Up @@ -626,6 +626,18 @@ func (k *K8sInstaller) listVersions() error {
return err
}

func getChainingMode(values map[string]interface{}) string {
cni, ok := values["cni"].(map[string]interface{})
if !ok {
return ""
}
chainingMode, ok := cni["chainingMode"].(string)
if !ok {
return ""
}
return chainingMode
}

func (k *K8sInstaller) preinstall(ctx context.Context) error {
if err := k.autodetectAndValidate(ctx); err != nil {
return err
Expand Down Expand Up @@ -658,7 +670,26 @@ func (k *K8sInstaller) preinstall(ctx context.Context) error {
return err
}
}
case k8s.KindEKS:
helmValues, err := k.params.HelmOpts.MergeValues(getter.All(cli.New()))
if err != nil {
return err
}
chainingMode := getChainingMode(helmValues)

// Do not stop AWS DS if we are running in chaining mode
if chainingMode != "aws-cni" {
if _, err := k.client.GetDaemonSet(ctx, AwsNodeDaemonSetNamespace, AwsNodeDaemonSetName, metav1.GetOptions{}); err == nil {
k.Log("🔥 Patching the %q DaemonSet to evict its pods...", AwsNodeDaemonSetName)
patch := []byte(fmt.Sprintf(`{"spec":{"template":{"spec":{"nodeSelector":{"%s":"%s"}}}}}`, AwsNodeDaemonSetNodeSelectorKey, AwsNodeDaemonSetNodeSelectorValue))
if _, err := k.client.PatchDaemonSet(ctx, AwsNodeDaemonSetNamespace, AwsNodeDaemonSetName, types.StrategicMergePatchType, patch, metav1.PatchOptions{}); err != nil {
k.Log("❌ Unable to patch the %q DaemonSet", AwsNodeDaemonSetName)
return err
}
}
}
}

return nil
}

Expand Down Expand Up @@ -699,23 +730,6 @@ func (k *K8sInstaller) Install(ctx context.Context) error {
}

switch k.flavor.Kind {
case k8s.KindEKS:
cm, err := k.generateConfigMap()
if err != nil {
return err
}
// Do not stop AWS DS if we are running in chaining mode
if cm.Data["cni-chaining-mode"] != "aws-cni" {
if _, err := k.client.GetDaemonSet(ctx, AwsNodeDaemonSetNamespace, AwsNodeDaemonSetName, metav1.GetOptions{}); err == nil {
k.Log("🔥 Patching the %q DaemonSet to evict its pods...", AwsNodeDaemonSetName)
patch := []byte(fmt.Sprintf(`{"spec":{"template":{"spec":{"nodeSelector":{"%s":"%s"}}}}}`, AwsNodeDaemonSetNodeSelectorKey, AwsNodeDaemonSetNodeSelectorValue))
if _, err := k.client.PatchDaemonSet(ctx, AwsNodeDaemonSetNamespace, AwsNodeDaemonSetName, types.StrategicMergePatchType, patch, metav1.PatchOptions{}); err != nil {
k.Log("❌ Unable to patch the %q DaemonSet", AwsNodeDaemonSetName)
return err
}
}
}

case k8s.KindAKS:
// We only made the secret-based azure installation available in >= 1.12.0
// Introduced in https://github.com/cilium/cilium/pull/18010
Expand Down
32 changes: 32 additions & 0 deletions install/install_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// SPDX-License-Identifier: Apache-2.0
// Copyright Authors of Cilium

package install

import (
"testing"

"github.com/stretchr/testify/assert"
"helm.sh/helm/v3/pkg/cli"
"helm.sh/helm/v3/pkg/cli/values"
"helm.sh/helm/v3/pkg/getter"
)

func Test_getChainingMode(t *testing.T) {
assert.Equal(t, "", getChainingMode(nil))

opts := values.Options{}
vals, err := opts.MergeValues(getter.All(cli.New()))
assert.NoError(t, err)
assert.Equal(t, "", getChainingMode(vals))

opts = values.Options{JSONValues: []string{"cni={}"}}
vals, err = opts.MergeValues(getter.All(cli.New()))
assert.NoError(t, err)
assert.Equal(t, "", getChainingMode(vals))

opts = values.Options{Values: []string{"cni.chainingMode=aws-cni"}}
vals, err = opts.MergeValues(getter.All(cli.New()))
assert.NoError(t, err)
assert.Equal(t, "aws-cni", getChainingMode(vals))
}

0 comments on commit 493031b

Please sign in to comment.