EKS Node Group Support #179
Conversation
Signed-off-by: Christopher Hein <[email protected]>
Signed-off-by: Christopher Hein <[email protected]>
| AWSTemplateFormatVersion: 2010-09-09 | ||
| Description: 'AWS Service Operator - Amazon EKS Node Group (aso-1otq2cgd9)' | ||
| Parameters: | ||
| Namespace: |
There was a problem hiding this comment.
This is my first review of code in this repo so some of my understanding may be inaccurate.
Don't you have to Tag the operator created resource with Namespace, ResourceVersion, ResourceName i the CFN template?
|
|
||
| func (c *Operator) onAdd(obj interface{}) { | ||
| s := obj.(*awsV1alpha1.EKSNodeGroup).DeepCopy() | ||
| if s.Status.ResourceStatus == "" || s.Status.ResourceStatus == "DELETE_COMPLETE" { |
There was a problem hiding this comment.
When can a NodeGroup resource that was marked as DELETE_COMPLETE come back in the queue to invoke onAdd?
| - key: nodeGroupName | ||
| type: resourceName | ||
| description: | | ||
| NodeGroupName is the name of the EKS Node Group to be created. |
There was a problem hiding this comment.
We can't have multiple node groups in a cluster with the same name, right?
| return nil, err | ||
| } | ||
|
|
||
| resourceCopy := resource.DeepCopy() |
There was a problem hiding this comment.
Is there a reason to create a deep copy instead updating fields inline?
| } | ||
| } | ||
|
|
||
| if name != "" && namespace != "" { |
There was a problem hiding this comment.
Where do the messages sent for CloudFormation resources get filtered out? I'm guessing this code needs to be invoked only for the Stack event i.e. ResourceType == AWS::CloudFormation::Stack
|
|
||
| // EKSNodeGroupStatus holds the status of the Cloudformation template | ||
| type EKSNodeGroupStatus struct { | ||
| ResourceStatus string `json:"resourceStatus"` |
There was a problem hiding this comment.
Should this include current size of the ASG as well? Say I have CA deployed to the cluster and it's scaling up/down the instances. It would be nice for the describe to return current capacity of the node grop
| } | ||
| if helpers.IsStackComplete(oo.Status.ResourceStatus, false) && !reflect.DeepEqual(oo.Spec, no.Spec) { | ||
| cft := New(c.config, oo, c.topicARN) | ||
| output, err := cft.UpdateStack(no) |
There was a problem hiding this comment.
If CA has changed the current size of the ASG out of band of CFN stack, can calling update undo that change?
| return err | ||
| } | ||
| config.Recorder.AnnotatedEventf(obj, annotations, corev1.EventTypeWarning, strcase.ToCamel(strings.ToLower(msg.ParsedMessage["ResourceStatus"])), msg.ParsedMessage["ResourceStatusReason"]) | ||
| err = incrementRollbackCount(config, name, namespace) |
There was a problem hiding this comment.
Why are we incrementing rollback count on DELETE_COMPLETE event?
Issue #, if available: #109
Description of changes:
Adds the ability to describe an EKSNodeGroup using a CRD and will automatically deploy the ASG. You currently will still need to add the
IAMrole intoconfigmaps/aws-authto allow it to join but you can get this by using:Definition
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.