Skip to content
This repository has been archived by the owner on Dec 26, 2022. It is now read-only.

Use nomad-native attribute for host IP #1906

Merged
4 commits merged into from
Aug 18, 2022
Merged

Conversation

ghost
Copy link

@ghost ghost commented Aug 18, 2022

Which issue does this PR correspond to?

What changes does this PR make to Grapl? Why?

⚠️ This will break anyone's local environment using Nomad prior to 1.3.2. ⚠️
This switches us to using Nomad's native attribute to get the host IP since Nomad 1.3.2 has been out for over a month

Now that hashicorp/nomad#12817 is merged in as
of Nomad 1.3.2, we should stop using the hax to get the host IP and
instead use Nomad's native method to get the host IP.

How were these changes tested?

Tested locally with make up and checking Jaeger, regression tests may also pick up any issues but I'm not sure if Pulumi/Nomad will pick it up.

Now that hashicorp/nomad#12817 is merged in as
of Nomad 1.3.2, we should stop using the hax to get the host IP and
instead use Nomad's native method to get the host IP.
@ghost ghost self-assigned this Aug 18, 2022
@ghost ghost requested a review from christophermaier as a code owner August 18, 2022 00:51
 Conflicts:
	nomad/grapl-core.nomad
	pulumi/grapl/__main__.py
Copy link
Contributor

@christophermaier christophermaier left a comment

Choose a reason for hiding this comment

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

Looks good, with one question.

@@ -0,0 +1,12 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this supposed to have been committed?

Copy link
Author

Choose a reason for hiding this comment

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

Nope, removing now

@ghost ghost force-pushed the twunderlich/switch-to-nomad-native-ip branch from 1bd7769 to 1e139a7 Compare August 18, 2022 14:04
This fixes one issue where the Nomad job doesn't accept any variables
which results in a KeyError, and speeds up cases where we are not
passing in variables
@codecov
Copy link

codecov bot commented Aug 18, 2022

Codecov Report

Merging #1906 (1e8ecfd) into main (3e25a97) will increase coverage by 0.01%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main    #1906      +/-   ##
==========================================
+ Coverage   43.32%   43.34%   +0.01%     
==========================================
  Files         432      432              
  Lines       11653    11652       -1     
  Branches       23       23              
==========================================
+ Hits         5049     5050       +1     
+ Misses       6589     6587       -2     
  Partials       15       15              
Impacted Files Coverage Δ
pulumi/grapl/__main__.py 0.00% <ø> (ø)
pulumi/infra/config.py 0.00% <ø> (ø)
pulumi/infra/nomad_job.py 0.00% <0.00%> (ø)
pulumi/rust_integration_tests/__main__.py 0.00% <ø> (ø)
src/rust/sysmon-parser/src/util.rs 59.72% <0.00%> (+0.46%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@ghost ghost merged commit 2a65bb2 into main Aug 18, 2022
@ghost ghost deleted the twunderlich/switch-to-nomad-native-ip branch August 18, 2022 15:17
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants