-
Notifications
You must be signed in to change notification settings - Fork 109
[DFSM]Using .login_nodes_keys_sync_file to be used during Init and Update phase of the clusters #2678
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
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #2678 +/- ##
========================================
Coverage 76.48% 76.48%
========================================
Files 22 22
Lines 2220 2220
========================================
Hits 1698 1698
Misses 522 522
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| default['cluster']['shared_dir_compute'] = node['cluster']['shared_dir'] | ||
| default['cluster']['shared_dir_head'] = node['cluster']['shared_dir'] | ||
| default['cluster']['shared_dir_login'] = node['cluster']['shared_dir_login_nodes'] | ||
| default['cluster']['shared_login_nodes_keys_sync_file'] = "#{node['cluster']['shared_dir_login_nodes']}/.login_nodes_keys_sync_file" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank yo for adding this attribute!
| ::FileUtils.cp_r(node['cluster']['shared_storages_mapping_path'], node['cluster']['previous_shared_storages_mapping_path'], remove_destination: true) | ||
|
|
||
| Chef::Log.info("Updating #{login_node_keys_sync_file} during #{node['cluster']['node_type']} update") | ||
| write_sync_file(login_node_keys_sync_file) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fix would address the issue, but we should do something easier: in this sync file we should not even check the cluster config version at all because it's enough for it to be there to signal that the head node already created the keys. In the offending PRs we included the check of the cluster config version to replicate an approach that was equivalent to the sync file used by the head node to signal changes in the config file. In this case though, the check of the cluster config version is redudant and can be avoided.
41bbcfa to
167f3b5
Compare
…date phase of the clusters
Description of changes
[DFSM]Using .login_nodes_keys_sync_file to be used during Init and Update phase of the clusters
Bug:
Introduced in #2671 and #2672
The file we create
/opt/parallelcluster/shared_login_nodes/.login_nodes_keys_sync_fileas part of sync during cluster never gets updated when we Stop-Start the Cluster.This file needs to be updated or any new Login Nodes which are launched after update of the Cluster, goes through the Init phase and wait for the content to be the latest.
Tests
develop #2677
References
Checklist
developadd the branch name as prefix in the PR title (e.g.[release-3.6]).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.