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.

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

Add Cluster-Autoscaling IAM #268

Merged
merged 1 commit into from
Oct 18, 2018
Merged

Conversation

Lazyshot
Copy link

Adds the ability and flag for requesting the necessary
policy for the cluster-autoscaler application. This will
be revisited once the add-on work is done.

Issue #170

Checklist

  • Code compiles correctly (i.e make build)
  • Added tests that cover your change (if possible)
  • All tests passing (i.e. make test)

@@ -99,6 +99,7 @@ func createClusterCmd() *cobra.Command {

fs.BoolVar(&cfg.Addons.WithIAM.PolicyAmazonEC2ContainerRegistryPowerUser, "full-ecr-access", false, "enable full access to ECR")
fs.BoolVar(&cfg.Addons.Storage, "storage-class", true, "if true (default) then a default StorageClass of type gp2 provisioned by EBS will be created")
fs.BoolVar(&cfg.Addons.WithIAM.AutoScalingGroup, "auto-scaling-policy", false, "enable iam policy dependency for cluster-autoscaler")

This comment was marked as abuse.

Copy link
Contributor

@errordeveloper errordeveloper left a comment

Choose a reason for hiding this comment

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

LGTM overall, but please apply suggested changes 👍

Some docs on installing autoscaler itself would help. A blog post would be very neat, or perhaps there's some simple documentation we could link to?

@@ -132,5 +132,18 @@ func (n *NodeGroupResourceSet) addResourcesForIAM() {
},
)

if n.spec.Addons.WithIAM.AutoScalingGroup {
n.rs.attachAllowPolicy("AutoScalingControl", refIR, "*",

This comment was marked as abuse.

@@ -89,4 +89,5 @@ type ClusterAddons struct {
// AddonIAM provides an addon for the AWS IAM integration
type AddonIAM struct {
PolicyAmazonEC2ContainerRegistryPowerUser bool
AutoScalingGroup bool

This comment was marked as abuse.

@Lazyshot
Copy link
Author

Naming consistency makes a lot of sense here, that was the biggest thing I was struggling with.

@errordeveloper
Copy link
Contributor

@Lazyshot would you mind to rebase and squash please?

@Lazyshot Lazyshot force-pushed the feature/cluster-autoscaler-iam branch from d75e954 to f216ac9 Compare October 17, 2018 16:21
@errordeveloper
Copy link
Contributor

There seem to be conflicts...

@errordeveloper
Copy link
Contributor

I am keen to get this in soon and cut 0.1.7!

@Lazyshot
Copy link
Author

I'll rebase again from master.

Adds the ability and flag for requesting the necessary
policy for the cluster-autoscaler application. This will
be revisited once the add-on work is done.

Issue #170
@Lazyshot Lazyshot force-pushed the feature/cluster-autoscaler-iam branch from f216ac9 to 7ae8543 Compare October 17, 2018 18:55
@errordeveloper errordeveloper merged commit 5e5e4d8 into master Oct 18, 2018
@errordeveloper errordeveloper deleted the feature/cluster-autoscaler-iam branch October 18, 2018 05:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants