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

Don't replace service account with update-aws-node #2459

Merged
merged 3 commits into from Jul 21, 2020
Merged

Don't replace service account with update-aws-node #2459

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6ab34f3
Don't replace service account with update-aws-node
michaelbeaumont Jul 21, 2020
22890e9
Slight refactor of UpdateAWSNode
michaelbeaumont Jul 21, 2020
d1cffe6
Add log message when skipping service account
michaelbeaumont Jul 21, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
28 changes: 20 additions & 8 deletions pkg/addons/default/aws_node.go
View file
Open in desktop
Expand Up @@ -45,7 +45,8 @@ func UpdateAWSNode(rawClient kubernetes.RawClientInterface, region string, plan
if err != nil {
return false, err
}
if resource.GVK.Kind == "DaemonSet" {
switch resource.GVK.Kind {
case "DaemonSet":
daemonSet, ok := resource.Info.Object.(*appsv1.DaemonSet)
if !ok {
return false, fmt.Errorf("expected type %T; got %T", &appsv1.Deployment{}, resource.Info.Object)
Expand All @@ -67,13 +68,24 @@ func UpdateAWSNode(rawClient kubernetes.RawClientInterface, region string, plan
if err != nil {
return false, err
}
}

if resource.GVK.Kind == "CustomResourceDefinition" && plan {
// eniconfigs.crd.k8s.amazonaws.com CRD is only partially defined in the
// manifest, and causes a range of issue in plan mode, we can skip it
logger.Info(resource.LogAction(plan, "replaced"))
continue
case "CustomResourceDefinition":
if plan {
// eniconfigs.crd.k8s.amazonaws.com CRD is only partially defined in the
// manifest, and causes a range of issue in plan mode, we can skip it
logger.Info(resource.LogAction(plan, "replaced"))
continue
}
case "ServiceAccount":
// Leave service account if it exists
// to avoid overwriting annotations
_, exists, err := resource.Get()
if err != nil {
return false, err
}
if exists {
logger.Info(resource.LogAction(plan, "skipped existing"))
continue
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be consistent to have a log message indicating that it was skipped, somewhat like the "pretend it was replaced" log message for CustomResourceDefinition directly above.

}
}

status, err := resource.CreateOrReplace(plan)
Expand Down
6 changes: 5 additions & 1 deletion pkg/addons/default/aws_node_test.go
View file
Open in desktop
Expand Up @@ -3,6 +3,7 @@ package defaultaddons_test
import (
. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
. "github.com/onsi/gomega/gstruct"

. "github.com/weaveworks/eksctl/pkg/addons/default"

Expand Down Expand Up @@ -56,7 +57,10 @@ var _ = Describe("default addons - aws-node", func() {

_, err := UpdateAWSNode(rawClient, "eu-west-1", false)
Expect(err).ToNot(HaveOccurred())
Expect(rawClient.Collection.UpdatedItems()).To(HaveLen(4))
Expect(rawClient.Collection.UpdatedItems()).To(HaveLen(3))
Expect(rawClient.Collection.UpdatedItems()).ToNot(ContainElement(PointTo(MatchFields(IgnoreMissing|IgnoreExtras, Fields{
"TypeMeta": MatchFields(IgnoreMissing|IgnoreExtras, Fields{"Kind": Equal("ServiceAccount")}),
}))))
Expect(rawClient.Collection.CreatedItems()).To(HaveLen(10))

rawClient.ClientSetUseUpdatedObjects = true // for verification of updated objects
Expand Down