Skip to content

Conversation

@cjmakin
Copy link
Contributor

@cjmakin cjmakin commented Jul 23, 2024

Description of changes

These changes enable DCV on login nodes. A user can now connect to a login node with DCV using dcv-connect and --login-node-ip parameter.

  • Allow DCV instance metadata access when enabled on login nodes.
  • Change dcv sessionID filename to include the hostname of the instance. This is to enable multiple sessionID files within the shared directory.
  • Update dcv.rb to configure dcv when enabled on login nodes.
  • Update supervisord to include pcluster_dcv_authenticator on login nodes.
  • Support DCV cloudwatch logs:
    • Update cloudwatch_agent_config.json with login node dcv_enabled dna key.
    • Add dcv logs to login node log rotation.
  • Update CHANGELOG

Tests

Manual tests:

  • Verified able to connect to login node with DCV only when enabled.
  • Verified DCV cloudwatch logging.
  • Verified DCV authenticator.

ChefSpec + Kitchen

  • Update supervisord_config_spec to test proper supervisord configuration when dcv is enabled on login nodes.
  • Update log_rotation_spec to test proper log configuration when dcv enabled on login nodes.
  • Update kitchen.platform-config log rotation resource.

References

  • Link to impacted open issues.
  • Link to related PRs in other packages (i.e. cookbook, node).
  • Link to documentation useful to understand the changes.

Checklist

  • Make sure you are pointing to the right branch.
  • If you're creating a patch for a branch other than develop add the branch name as prefix in the PR title (e.g. [release-3.6]).
  • Check all commits' messages are clear, describing what and why vs how.
  • Make sure to have added unit tests or integration tests to cover the new/modified code.
  • Check if documentation is impacted by this change.

Please review the guidelines for contributing and Pull Request Instructions.

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

cjmakin added 7 commits July 22, 2024 07:59
* Updated `dcv.rb` to activate dcv on the login node.
* Updated `supervisord_config` to set `dcv_installed?` to be true when dcv is enabled on a login node.
* Updated `parallelcluster_supervisord.conf.erb` to include `pcluster_dcv_authenticator` when dcv is configured on a login node.
This is required to have multiple session files within the shared directory.
* Added `dcv_enabled` attribute to the login node log rotation in `kitchen.platform-config.yml`.
* Updated `log_rotation_spec` to include dcv logs for login nodes.
@cjmakin cjmakin requested a review from hgreebe July 23, 2024 11:52
@cjmakin cjmakin requested review from a team as code owners July 23, 2024 11:52
@hgreebe
Copy link
Contributor

hgreebe commented Jul 23, 2024

One of the checks is failing because of cookstyle offenses, you can fix this by running cookstyle -a .

@cjmakin cjmakin merged commit 0aaa259 into aws:develop Jul 23, 2024
hgreebe pushed a commit to hgreebe/aws-parallelcluster-cookbook that referenced this pull request Aug 22, 2024
* Update platform cookbook to support DCV on login nodes

* Updated `dcv.rb` to activate dcv on the login node.
* Updated `supervisord_config` to set `dcv_installed?` to be true when dcv is enabled on a login node.
* Updated `parallelcluster_supervisord.conf.erb` to include `pcluster_dcv_authenticator` when dcv is configured on a login node.

* Modify _dcv_common to support DCV on login node

* Update cloudwatch config DCV logs to include login nodes

* Allow DCV instance metadata access when enabled on login node

* Rename the DCV sessionID file to be the instance hostname

This is required to have multiple session files within the shared directory.

* Update supervisord_config_spec to test DCV on login node

* Update kitchen test and spec tests for login node dcv

* Added `dcv_enabled` attribute to the login node log rotation in `kitchen.platform-config.yml`.
* Updated `log_rotation_spec` to include dcv logs for login nodes.

* Update CHANGELOG
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