ec2: shellcheck fixes

[r-vdp]

Description of changes

Fix errors when enabling shellcheck on systemd service scripts (see #311394)

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
  • sandbox = relaxed
  • sandbox = true
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
• Fits CONTRIBUTING.md.

---

Add a 👍 reaction to pull requests you find important.

[pbsds]

@ofborg test ec2-nixops

[pbsds]

ec2-nixops has failed twice on ofborg, but succeeds for me locally. Any ideas?

https://logs.ofborg.org/?key=nixos/nixpkgs.340489&attempt_id=708d5437-709b-43b5-9e41-f3c56c38bceb

@ofborg test ec2-nixops

[r-vdp]

I will try to take a look later today, I think it also worked for me locally when I made this commit (several months ago, things might have changed on master in the meantime).

I used to have some machines on AWS, which is why I ran into this in the first place, but I have since moved to another platform.

[Mic92]

ec2-nixops has failed twice on ofborg, but succeeds for me locally. Any ideas?

https://logs.ofborg.org/?key=nixos/nixpkgs.340489&attempt_id=708d5437-709b-43b5-9e41-f3c56c38bceb

@ofborg test ec2-nixops

I believe this plugin was also recently removed: #343727

[Mic92]

Ok. no. this was just the aws plugin.

[pbsds]

@ofborg test ec2-nixops

[r-vdp]

This test also fails on master for me when building it locally, I couldn't get it to work.

[pbsds]

able to reproduce the issue.

I checked with the shellcheck rationale, diff LGTM!

[arianvp]

Urgh this actually broke SSH access for our AWS images. I wish I was tagged for review on this. I would've tested it. I am marked as maintainer for amazon-image but apparently not for ec2-data. I'll make a PR to make sure I get tagged

[arianvp]

I will try to allocate some time to write / fix a NixOS test that would've made this a channel blocker

[pbsds]

Thank you for following this up. Consider adding yourself to CODEOWNERS as well to ensure the ping, since nixos PRs commonly don't ping meta.maintainers due to a lack of tooling. It should be a nicer experience and possible for anyone as of #347610, and be more encouraged.

[nixos-discourse]

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/breaking-changes-announcement-for-unstable/17574/63

[arianvp]

I'm trying to retro why this ever made it past a channel bump and I don't understand why. ec2-nixops (Which is just a badly named test as it doesn't do anything NixOps specific ; it just tests SSH access) is a channel-blocking test and it failed on this PR so I expect it also failed on Hydra. That the test was failing was remarked by multiple people in review and then this was merged anyway. This is not the part I'm interested in though. The part I'm interested in is why our automation didn't catch the issue after merge?

When it got merged I would expect the test to have failed on Hydra and block the channel. Why did this not happen?

[arianvp]

Ah because it's not in the tested job. I will make a PR to add it. I can also rename the test to be more descriptive