[Validators] Add validator ExtraChefAttributesValidator to validate format of ExtraChefAttributes and notify the user about downsides of in_place_update_on_fleet_enabled=False

[gmarciani]

Description of changes

This PR mitigates the performance degradation reported in #6449

Add validator ExtraChefAttributesValidator to warn the user against the consequences of disabling in-place updates through DevSettings, i.e. when the chef attribute in_place_update_on_fleet_enabled is set to False.

We introduced such new attribute in aws/aws-parallelcluster-cookbook#3040
See that PR for details about what the cookbook does when cfnhup is disabled.
Such attribute takes effect at config time (cluster creation/update, not at build image).

The validator does what follows:

1. validates that the attribute cluster.in_place_update_on_fleet_enabled is a boolean value.
2. if the attribute cluster.in_place_update_on_fleet_enabled is false, returns a warning to notify the user that disabling cfn-hup has consequences on the cluster update behavior.

User Experience

[UseCase 1] in-place updates enabled (implicitly)

By omitting the chef attribute in_place_update_on_fleet_enabled, cfn-hup is enabled. No warning are returned by the CLI

[UseCase 2] in-place updates enabled (explicitly)

By setting in_place_update_on_fleet_enabled to True, cfn-hup is enabled. No warning are returned by the CLI.

Config:

DevSettings:
  Cookbook:
    ExtraChefAttributes: |
      { "cluster" : { "in_place_update_on_fleet_enabled" : "true" } }

[UseCase 3] in-place updates disabled

When the cluster is created with the following attribute, cluster creation/update succeeds with a warning:

Config:

DevSettings:
  Cookbook:
    ExtraChefAttributes: |
      { "cluster" : { "in_place_update_on_fleet_enabled" : "false" } }

Warning:

    {
      "level": "WARNING",
      "type": "ExtraChefAttributesValidator",
      "message": "When in-place updates are disabled, cluster updates are applied by replacing compute and login nodes according to the selected QueueUpdateStrategy."
    }

[UseCase 4]: invalid value for the attribute

When the user provides an invalid value for in_place_update_on_fleet_enabled (i.e.: not boolean), cluster creation/update fails with error.

Config:

DevSettings:
    ExtraChefAttributes: |
      { "cluster" : { "in_place_update_on_fleet_enabled" : "XXXX" } }

Error:

    {
      "level": "ERROR",
      "type": "ExtraChefAttributesValidator",
      "message": "Invalid value in DevSettings/Cookbook/ExtraChefAttributes: attribute 'in_place_update_on_fleet_enabled' must be a boolean value."
    }

[UseCase 5]: invalid value for ExtraChefAttributes

When the user provides an invalid JSON for ExtraChefAttributes, cluster creation/update fails with a schema validation error (existing logic, mentioning for completeness).

Config:

DevSettings:
  Cookbook:
    ExtraChefAttributes: |
      {INVALID JSON}

{
  "configurationValidationErrors": [
    {
      "level": "ERROR",
      "type": "ConfigSchemaValidator",
      "message": "[('DevSettings', {'Cookbook': {'ExtraChefAttributes': [\"'{INVALID JSON}\\n' is invalid\"]}})]"
    }
  ],
  "message": "Invalid cluster configuration."
}

Tests

• Unit tests (existing and new ones)
• Manually validated all the use cases reported in User Experience.

Q&A

1. Why not testing the update of the new attribute?
   Updates to ExtraChefAttributes have never been supported as per update policy here

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[himani2411]

Have we tested this use-case? where we create a cluster with in_place_update_on_fleet_enabled as false and then try to update the cluster to use in_place_update_on_fleet_enabled as true?

[gmarciani]

Have we tested this use-case? where we create a cluster with in_place_update_on_fleet_enabled as false and then try to update the cluster to use in_place_update_on_fleet_enabled as true?

Updates to ExtraChefAttributes have never been supported as per update policy here

Added to Q&A section.

[himani2411]

[Non-blocking] Seems redundant to convert a string to a string

[gmarciani]

that is to protect against None values.

If you remove the string conversion and pass None as input, the function would throw the error AttributeError: 'NoneType' object has no attribute 'lower'

[himani2411]

LGTM!