Merged
Conversation
This means that if the server's IP address changes then the clients should still be able to find the new IP address as long as the DNS record is up to date.
Now updating any of the Nomad variable resources correctly triggers the command to rerun.
|
✔️ e2e986b...4e0b717 - Conventional commits check succeeded. |
WalkthroughUpdates Nomad ACL policy to add allocation lifecycle management capability, modifies policy application command description in infrastructure-as-code, and replaces dynamic IP templates with a fixed server hostname in Nomad configuration. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Collaborator
🍹
|
|
🍹 The Update (preview) for holochain/nomad-server/nomad-server (at 4e0b717) was successful. ✨ Neo ExplanationUpdated Nomad configuration and/or job runner policy files are being pushed to the server, triggering a full re-run of the setup commands including a Nomad service restart — expect a brief Nomad downtime during the apply.Root Cause AnalysisThe Nomad configuration files and/or job runner policy files have been modified locally. Both Dependency Chain
Each resource is doing a delete-and-replace (not an in-place update), meaning Pulumi will tear down the old remote command records and re-execute them fresh. Risk AnalysisThe Nomad service will be restarted as part of this update ( Resource Changes Name Type Operation
+- copy-job-runner-policy command:remote:CopyToRemote replaced
+- start-nomad-service command:remote:Command replaced
+- add-nomad-jobs-vars command:remote:Command replaced
+- chown-etc-nomad-dir-before-server-cert command:remote:Command replaced
+- apply-job-runner-policy command:remote:Command replaced
+- copy-nomad-config command:remote:CopyToRemote replaced
+- chown-etc-nomad-dir command:remote:Command replaced
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
nomad.hcl (1)
8-9: Consider centralizing the advertised hostname to avoid drift.This FQDN is now repeated in multiple places (
nomad.hclLine 8-9,nomad-client.hclLine 7-10, and certificate generation inmain.goLine 170). Consider sourcing it from one config value to prevent future mismatch.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nomad.hcl` around lines 8 - 9, Centralize the advertised hostname by introducing a single configuration value (e.g., advertised_host) and replace the duplicated literals used for the http and rpc entries in nomad.hcl and the client entries in nomad-client.hcl with references to that config value; update main.go’s certificate generation site (the block that builds the certificate SANs / common name around Line ~170) to read the same advertised_host instead of hardcoding "nomad-server-01.holochain.org", and ensure the new config is loaded (env var or shared config loader) so all three places (nomad.hcl http/rpc, nomad-client.hcl entries, and main.go cert generation) derive the hostname from the single source.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@nomad.hcl`:
- Around line 8-9: Centralize the advertised hostname by introducing a single
configuration value (e.g., advertised_host) and replace the duplicated literals
used for the http and rpc entries in nomad.hcl and the client entries in
nomad-client.hcl with references to that config value; update main.go’s
certificate generation site (the block that builds the certificate SANs / common
name around Line ~170) to read the same advertised_host instead of hardcoding
"nomad-server-01.holochain.org", and ensure the new config is loaded (env var or
shared config loader) so all three places (nomad.hcl http/rpc, nomad-client.hcl
entries, and main.go cert generation) derive the hostname from the single
source.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: df6513f7-9f7f-4a7c-9f52-d5b2c56508f0
📒 Files selected for processing (3)
job-runner.policy.hclmain.gonomad.hcl
veeso
approved these changes
Mar 30, 2026
ThetaSinner
approved these changes
Mar 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Takes over from #7, so that we are not re-applying multiple times.
This PR:
alloc-lifecyclecapability to the GitHub runner token policy which is required to allow the GitHub CI to stop/kill running Nomad allocations.Summary by CodeRabbit