Skip to content

Commit

Permalink
Add unmanaged CNI for FLC (#5262)
Browse files Browse the repository at this point in the history
  • Loading branch information
chrisdoherty4 authored Mar 24, 2023
1 parent 5953dd9 commit 9acc4c1
Show file tree
Hide file tree
Showing 8 changed files with 357 additions and 16 deletions.
3 changes: 2 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ GO ?= $(GO_VERSION)/go
GO_TEST ?= $(GO) test
# A regular expression defining what packages to exclude from the unit-test recipe.
UNIT_TEST_PACKAGE_EXCLUSION_REGEX ?=mocks$
UNIT_TEST_PACKAGES ?= $$($(GO) list ./... | grep -vE "$(UNIT_TEST_PACKAGE_EXCLUSION_REGEX)")

## ensure local execution uses the 'main' branch bundle
BRANCH_NAME?=main
Expand Down Expand Up @@ -422,7 +423,7 @@ unit-test: ## Run unit tests
unit-test: $(SETUP_ENVTEST)
unit-test: KUBEBUILDER_ASSETS ?= $(shell $(SETUP_ENVTEST) use --use-env -p path --arch $(GO_ARCH) $(KUBEBUILDER_ENVTEST_KUBERNETES_VERSION))
unit-test:
KUBEBUILDER_ASSETS="$(KUBEBUILDER_ASSETS)" $(GO_TEST) $$($(GO) list ./... | grep -vE "$(UNIT_TEST_PACKAGE_EXCLUSION_REGEX)") -cover -tags "$(BUILD_TAGS)" $(GO_TEST_FLAGS)
KUBEBUILDER_ASSETS="$(KUBEBUILDER_ASSETS)" $(GO_TEST) $(UNIT_TEST_PACKAGES) -cover -tags "$(BUILD_TAGS)" $(GO_TEST_FLAGS)


# unit-test-patch is a convenience target that restricts test runs to modified
Expand Down
15 changes: 15 additions & 0 deletions pkg/api/v1alpha1/cluster_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,21 @@ func validateImmutableFieldsCluster(new, old *Cluster) field.ErrorList {
field.Forbidden(specPath.Child("clusterNetwork", "dns"), "field is immutable"))
}

// We don't want users to be able to toggle off SkipUpgrade until we've understood the
// implications so we are temporarily disallowing it.

oCNI := old.Spec.ClusterNetwork.CNIConfig
nCNI := new.Spec.ClusterNetwork.CNIConfig
if oCNI != nil && oCNI.Cilium != nil && !oCNI.Cilium.IsManaged() && nCNI.Cilium.IsManaged() {
allErrs = append(
allErrs,
field.Forbidden(
specPath.Child("clusterNetwork", "cniConfig", "cilium", "skipUpgrade"),
"cannot toggle off skipUpgrade once enabled",
),
)
}

if !new.Spec.ClusterNetwork.Nodes.Equal(old.Spec.ClusterNetwork.Nodes) {
allErrs = append(
allErrs,
Expand Down
82 changes: 82 additions & 0 deletions pkg/api/v1alpha1/cluster_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1845,6 +1845,88 @@ func TestClusterValidateUpdateLabelTaintsWNTinkerbellRequest(t *testing.T) {
g.Expect(err).To(MatchError(ContainSubstring("spec.WorkerNodeConfiguration.taints: Forbidden: field is immutable")))
}

func TestClusterValidateUpdateSkipUpgradeImmutability(t *testing.T) {
tests := []struct {
Name string
Old *v1alpha1.Cluster
New *v1alpha1.Cluster
Error bool
}{
{
Name: "NilToFalse",
Old: createCluster(func(c *v1alpha1.Cluster) {
c.Spec.ClusterNetwork.CNIConfig.Cilium.SkipUpgrade = nil
}),
New: createCluster(func(c *v1alpha1.Cluster) {
c.Spec.ClusterNetwork.CNIConfig.Cilium.SkipUpgrade = ptr.Bool(false)
}),
},
{
Name: "FalseToNil",
Old: createCluster(func(c *v1alpha1.Cluster) {
c.Spec.ClusterNetwork.CNIConfig.Cilium.SkipUpgrade = ptr.Bool(false)
}),
New: createCluster(func(c *v1alpha1.Cluster) {
c.Spec.ClusterNetwork.CNIConfig.Cilium.SkipUpgrade = nil
}),
},
{
Name: "NilToTrue",
Old: createCluster(func(c *v1alpha1.Cluster) {
c.Spec.ClusterNetwork.CNIConfig.Cilium.SkipUpgrade = nil
}),
New: createCluster(func(c *v1alpha1.Cluster) {
c.Spec.ClusterNetwork.CNIConfig.Cilium.SkipUpgrade = ptr.Bool(true)
}),
},
{
Name: "FalseToTrue",
Old: createCluster(func(c *v1alpha1.Cluster) {
c.Spec.ClusterNetwork.CNIConfig.Cilium.SkipUpgrade = ptr.Bool(false)
}),
New: createCluster(func(c *v1alpha1.Cluster) {
c.Spec.ClusterNetwork.CNIConfig.Cilium.SkipUpgrade = ptr.Bool(true)
}),
},
{
Name: "TrueToNil",
Old: createCluster(func(c *v1alpha1.Cluster) {
c.Spec.ClusterNetwork.CNIConfig.Cilium.SkipUpgrade = ptr.Bool(true)
}),
New: createCluster(func(c *v1alpha1.Cluster) {
c.Spec.ClusterNetwork.CNIConfig.Cilium.SkipUpgrade = nil
}),
Error: true,
},
{
Name: "TrueToFalse",
Old: createCluster(func(c *v1alpha1.Cluster) {
c.Spec.ClusterNetwork.CNIConfig.Cilium.SkipUpgrade = ptr.Bool(true)
}),
New: createCluster(func(c *v1alpha1.Cluster) {
c.Spec.ClusterNetwork.CNIConfig.Cilium.SkipUpgrade = ptr.Bool(false)
}),
Error: true,
},
}

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

err := tc.New.ValidateUpdate(tc.Old)
if !tc.Error {
g.Expect(err).To(Succeed())
} else {
g.Expect(err).To(MatchError(ContainSubstring(
"spec.clusterNetwork.cniConfig.cilium.skipUpgrade: Forbidden: cannot toggle " +
"off skipUpgrade once enabled",
)))
}
})
}
}

func newCluster(opts ...func(*v1alpha1.Cluster)) *v1alpha1.Cluster {
c := createCluster()
for _, o := range opts {
Expand Down
16 changes: 12 additions & 4 deletions pkg/networking/cilium/installation.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package cilium

import (
"strings"

appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
)
Expand All @@ -12,9 +14,15 @@ type Installation struct {
ConfigMap *corev1.ConfigMap
}

// Installed determines if all Cilium components are present.
// If the ConfigMap doesn't exist we still considered Cilium is installed.
// The installation might not be complete but it can be functional.
// Installed determines if all EKS-A Embedded Cilium components are present. It identifies
// EKS-A Embedded Cilium by the image name. If the ConfigMap doesn't exist we still considered
// Cilium is installed. The installation might not be complete but it can be functional.
func (i Installation) Installed() bool {
return i.DaemonSet != nil && i.Operator != nil
var isEKSACilium bool
if i.DaemonSet != nil {
for _, c := range i.DaemonSet.Spec.Template.Spec.Containers {
isEKSACilium = isEKSACilium || strings.Contains(c.Image, "eksa")
}
}
return i.DaemonSet != nil && i.Operator != nil && isEKSACilium
}
33 changes: 31 additions & 2 deletions pkg/networking/cilium/installation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (

. "github.com/onsi/gomega"
appsv1 "k8s.io/api/apps/v1"
v1 "k8s.io/api/core/v1"

"github.com/aws/eks-anywhere/pkg/networking/cilium"
)
Expand All @@ -18,8 +19,18 @@ func TestInstallationInstalled(t *testing.T) {
{
name: "installed",
installation: cilium.Installation{
DaemonSet: &appsv1.DaemonSet{},
Operator: &appsv1.Deployment{},
DaemonSet: &appsv1.DaemonSet{
Spec: appsv1.DaemonSetSpec{
Template: v1.PodTemplateSpec{
Spec: v1.PodSpec{
Containers: []v1.Container{
{Image: "cilium-eksa"},
},
},
},
},
},
Operator: &appsv1.Deployment{},
},
want: true,
},
Expand All @@ -30,6 +41,24 @@ func TestInstallationInstalled(t *testing.T) {
},
want: false,
},
{
name: "ds not installed with eksa cilium",
installation: cilium.Installation{
DaemonSet: &appsv1.DaemonSet{
Spec: appsv1.DaemonSetSpec{
Template: v1.PodTemplateSpec{
Spec: v1.PodSpec{
Containers: []v1.Container{
{Image: "cilium"},
},
},
},
},
},
Operator: &appsv1.Deployment{},
},
want: false,
},
{
name: "operator not installed",
installation: cilium.Installation{
Expand Down
27 changes: 27 additions & 0 deletions pkg/networking/cilium/reconciler/marker.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package reconciler

import (
"context"

"github.com/aws/eks-anywhere/pkg/api/v1alpha1"
"github.com/aws/eks-anywhere/pkg/controller/clientutil"
)

// EKSACiliumInstalledAnnotation indicates a cluster has previously been observed to have
// EKS-A Cilium installed irrespective of whether its still installed.
const EKSACiliumInstalledAnnotation = "anywhere.eks.amazonaws.com/eksa-cilium"

// ciliumWasInstalled checks cluster for the EKSACiliumInstalledAnnotation.
func ciliumWasInstalled(ctx context.Context, cluster *v1alpha1.Cluster) bool {
if cluster.Annotations == nil {
return false
}
_, ok := cluster.Annotations[EKSACiliumInstalledAnnotation]
return ok
}

// markCiliumInstalled populates the EKSACiliumInstalledAnnotation on cluster. It may trigger
// anothe reconciliation event.
func markCiliumInstalled(ctx context.Context, cluster *v1alpha1.Cluster) {
clientutil.AddAnnotation(cluster, EKSACiliumInstalledAnnotation, "")
}
62 changes: 54 additions & 8 deletions pkg/networking/cilium/reconciler/reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package reconciler

import (
"context"
"fmt"
"time"

"github.com/go-logr/logr"
Expand Down Expand Up @@ -39,17 +40,62 @@ func New(templater Templater) *Reconciler {
}
}

// Reconcile takes the Cilium CNI in a cluster to the desired state defined in a cluster Spec
// It uses a controller.Result to indicate when requeues are needed
// Intended to be used in a kubernetes controller.
// Reconcile takes the Cilium CNI in a cluster to the desired state defined in a cluster Spec.
// It uses a controller.Result to indicate when requeues are needed. client is connected to the
// target Kubernetes cluster, not the management cluster.
func (r *Reconciler) Reconcile(ctx context.Context, logger logr.Logger, client client.Client, spec *cluster.Spec) (controller.Result, error) {
installation, err := getInstallation(ctx, client)
if err != nil {
return controller.Result{}, err
}

if !installation.Installed() {
return r.install(ctx, logger, client, spec)
// We use a marker to detect if EKS-A Cilium has ever been installed. If it has never been
// installed and isn't currently installed we always attempt to install it regardless of whether
// the user is skipping EKS-A Cilium management. This satsifies criteria for successful cluster
// creation.
//
// If EKS-A Cilium was previously installed, as denoted by the marker, we only want to
// manage it if its still installed and the user still wants us to manage the installation (as
// denoted by the API skip flag).
//
// In the event a user uninstalls EKS-A Cilium, updates the cluster spec to skip EKS-A Cilium
// management, then tries to upgrade, we will attempt to install EKS-A Cilium. This is because
// reconciliation has no operational context (create vs upgrade) and can only observe that no
// installation is present and there is no marker indicating it was ever present which is
// equivilent to a typical create scenario where we must install a CNI to satisfy cluster
// create success criteria.

// To accommodate upgrades of cluster cerated prior to introducing markers, we check for
// an existing installation and try to mark the cluster as having already had EKS-A
// Cilium installed.
if !ciliumWasInstalled(ctx, spec.Cluster) && installation.Installed() {
logger.Info(fmt.Sprintf(
"Cilium installed but missing %v annotation; applying annotation",
EKSACiliumInstalledAnnotation,
))
markCiliumInstalled(ctx, spec.Cluster)
}

ciliumCfg := spec.Cluster.Spec.ClusterNetwork.CNIConfig.Cilium

if !installation.Installed() &&
(ciliumCfg.IsManaged() || !ciliumWasInstalled(ctx, spec.Cluster)) {
if err := r.install(ctx, logger, client, spec); err != nil {
return controller.Result{}, err
}

logger.Info(fmt.Sprintf(
"Applying %v annotation to Cluster object",
EKSACiliumInstalledAnnotation,
))
markCiliumInstalled(ctx, spec.Cluster)

return controller.Result{}, nil
}

if !ciliumCfg.IsManaged() {
logger.Info("Cilium configured as unmanaged, skipping upgrade")
return controller.Result{}, nil
}

logger.Info("Cilium is already installed, checking if it needs upgrade")
Expand All @@ -75,13 +121,13 @@ func (r *Reconciler) Reconcile(ctx context.Context, logger logr.Logger, client c
return r.deletePreflightIfExists(ctx, client, spec)
}

func (r *Reconciler) install(ctx context.Context, log logr.Logger, client client.Client, spec *cluster.Spec) (controller.Result, error) {
func (r *Reconciler) install(ctx context.Context, log logr.Logger, client client.Client, spec *cluster.Spec) error {
log.Info("Installing Cilium")
if err := r.applyFullManifest(ctx, client, spec); err != nil {
return controller.Result{}, errors.Wrap(err, "installing Cilium")
return errors.Wrap(err, "installing Cilium")
}

return controller.Result{}, nil
return nil
}

func (r *Reconciler) upgrade(ctx context.Context, logger logr.Logger, client client.Client, installation *cilium.Installation, spec *cluster.Spec) (controller.Result, error) {
Expand Down
Loading

0 comments on commit 9acc4c1

Please sign in to comment.