Skip to content
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.

Sign up for GitHub

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

Jump to bottom

262 nix fix #263

Merged
merged 1 commit into from
Mar 7, 2024
Merged

262 nix fix #263

merged 1 commit into from
Mar 7, 2024

Conversation

andriy-safe-ai
Copy link
Contributor

Rewrote playbooks to install Nix on boot volume of login node. Removed socat. Set Flakes flag to false.

@andriy-safe-ai andriy-safe-ai self-assigned this Mar 6, 2024
@andriy-safe-ai andriy-safe-ai linked an issue Mar 6, 2024 that may be closed by this pull request
Nix Fix #262
Closed
steven-safeai
steven-safeai requested changes Mar 6, 2024
View reviewed changes
Copy link
Contributor

@steven-safeai steven-safeai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you leave a comment or a name where the /nix directory gets shared to everything as ro? I think I missed it in initial pass.

playbooks/install_nix.yml Outdated
- name: Get nix version
hosts: slurm_backup
hosts: login
become: true
tasks:
- shell: |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you leave a comment for this line:
"nix_version_on_system: "{{ (nix_version_output.stdout_lines[0].split(' '))[2] }}""

It looks fugly.
What if nix changes the way it's formatted it's output? Can it pass?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. There is no guarantee that we will be able to parse the version correctly if the output format changes.

I've rewritten it to just check if Nix is installed or not. Also removed the crazy parser line. Much cleaner too.

playbooks/install_nix.yml Show resolved Hide resolved
playbooks/install_nix.yml Outdated
fstype: none
opts: bind
state: mounted
- name: Install Nix
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Recursion? Looks weird

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the name from the task.

playbooks/install_nix.yml Outdated

- name: Link /var/run/nix and /data/nix/var/nix/daemon-socket dirs
hosts: slurm_backup
- hosts: login
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why don't these have names?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added names.

playbooks/install_nix.yml Outdated
# install nix
- name: Install Nix
hosts: slurm_backup
- hosts: bastion, slurm_backup, compute
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No name?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added name.

steven-safeai
steven-safeai reviewed Mar 7, 2024
View reviewed changes
playbooks/install_nix.yml Outdated
@@ -1,143 +1,110 @@
---

- name: Get nix version
hosts: slurm_backup
- name: Check whether is already installed
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whether nix is installed

steven-safeai
steven-safeai reviewed Mar 7, 2024
View reviewed changes
playbooks/uninstall_nix.yml

- name: Remove files and dirs created by Nix
hosts: slurm_backup
hosts: all
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confused I thought the files were only created on the login node then get shared to the others. Do they get physically chopped to all machines?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only shared but there is a /nix dir on all nodes that I need to remove. I included all since it wouldn't hurt to run the other plays.

@andriy-safe-ai
Copy link
Contributor Author

andriy-safe-ai commented Mar 7, 2024 •
edited
Loading

The latest changes (34baa44) have been tested in a dev cluster.

  • Tested installing (pass)
  • Tested uninstalling (pass)
  • Tested installing while Nix was already installed (pass)
  • Tested installing, deleting, updating packages (pass)

@andriy-safe-ai
Copy link
Contributor Author

andriy-safe-ai commented Mar 7, 2024 •
edited
Loading

@steven-basart
The nfs-client and nfs-server roles were not changed functionally. The only difference is that I added a newline to separate two plays in nfs-client/tasks/el.yml.

Steps I performed:

  1. SCP'd the nfs-client and nfs-server roles from prod to my local machine
  2. Performed a diff -r between the roles I obtained from production and those from the latest branch.

steven-safeai
steven-safeai approved these changes Mar 7, 2024
View reviewed changes
Copy link
Contributor

@steven-safeai steven-safeai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems good to me.

@andriy-safe-ai
Copy link
Contributor Author

Rebased to one commit. Merged.

@andriy-safe-ai andriy-safe-ai merged commit 5373aee into main Mar 7, 2024
@andriy-safe-ai andriy-safe-ai deleted the 262-nix-fix branch March 7, 2024 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@steven-safeai steven-safeai steven-safeai approved these changes

Assignees

@andriy-safe-ai andriy-safe-ai

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Nix Fix
2 participants
@andriy-safe-ai @steven-safeai