allow users to avoid aws instance not found spam

[grosser]

What type of PR is this?

/kind bug

What this PR does / why we need it:

allows users to mark nodes for "do not spam me about missing aws instance"

Which issue(s) this PR fixes:

fixes #6096

Special notes for your reviewer:

not ready to be merged, just for discussion

Does this PR introduce a user-facing change?

TODO

[drmorr0]

Can we add a test confirming this works correctly?

[grosser]

yes, I can add tests if anyone with merge permission can confirm this is a good approach

[drmorr0]

This seems fine to me, yes.

Rather: imo the ideal approach would be to just have that error be logged at a lower verbosity, but I think that's going to be non-trivial to accomplish given how the code is currently organized, so this seems fine as an alternate solution.

[grosser]

improved comment, added test

[drmorr0]

/approve
/lgtm

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: drmorr0, grosser

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• cluster-autoscaler/cloudprovider/aws/OWNERS [drmorr0]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[brydoncheyney]

k8s.io/cluster-autoscaler/enabled is not a valid annotation key - https://kubernetes.io/docs/concepts/overview/working-with-objects/annotations/#syntax-and-character-set - which expects only a single DNS prefix.

Attempting to manually add this annotation to an existing node will fail -

# nodes "ip-XXX-XXX-XXX-XXX.ec2.internal" was not valid:
# * metadata.annotations: Invalid value: "k8s.io/cluster-autoscaler/enabled": a qualified name must consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyName',  or 'my.name',  or '123-abc', regex used for validation is '([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]') with an optional DNS subdomain prefix and '/' (e.g. 'example.com/MyName')

Note that the use of the k8s.io/cluster-autoscaler/enabled tag is valid within AWS.

This could be fixed by using an alternative key format, such as k8s.io/cluster-autoscaler-enabled

I'll pull a PR together for this.

[grosser]

thx for checking/fixing! :)

…

On Mon, Nov 20, 2023 at 4:47 AM Brydon Cheyney ***@***.***> wrote: k8s.io/cluster-autoscaler/enabled is not a valid annotation key - https://kubernetes.io/docs/concepts/overview/working-with-objects/annotations/#syntax-and-character-set - which expects only a *single* DNS prefix. Attempting to manually add this annotation to an existing node will fail - # nodes "ip-XXX-XXX-XXX-XXX.ec2.internal" was not valid: # * metadata.annotations: Invalid value: "k8s.io/cluster-autoscaler/enabled": a qualified name must consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyName', or 'my.name', or '123-abc', regex used for validation is '([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]') with an optional DNS subdomain prefix and '/' (e.g. 'example.com/MyName') Note that the use of the k8s.io/cluster-autoscaler/enabled *tag* is valid within AWS. This could be fixed by using an alternative key format, such as k8s.io/cluster-autoscaler-enabled I'll pull a PR together for this. — Reply to this email directly, view it on GitHub <#6265 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAACYZ5UCSTVBJR4T5RQ4L3YFNGPVAVCNFSM6AAAAAA7FAKB4OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMJZGAYDENZYGE> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[scottblack1]

Curious why we have used node.annotation here instead of a label for example?

As far as I can see there is no easy way to automatically annotate nodes in EKS as this is not an available argument in the eks_bootstrap.sh.