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.

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

Fixups for running in switch mode #54

Closed
wants to merge 1 commit into from
Closed

Fixups for running in switch mode #54

wants to merge 1 commit into from

Conversation

leigh-thg
Copy link

I needed to make the following changes to get a VPN up and running in switch mode

  • Actually set the mode in tinc.conf
  • Skip errors on the initial check for an RSA key already being present. For leaf nodes the host file will not have been created yet, so the awk command fails. There may be a better way to accomplish this.

…e host file won't have been created for leaf nodes on the first pass
Copy link
Owner

@evrardjp evrardjp left a comment

Choose a reason for hiding this comment

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

What do you think of the following ?

@@ -60,6 +60,7 @@
command: awk '/^-----END RSA PUBLIC KEY-----$/' /etc/tinc/{{ tinc_netname }}/hosts/{{ inventory_hostname | replace('.','_') | replace('-','_') }}
changed_when: "public_key.stdout != '-----END RSA PUBLIC KEY-----'"
register: public_key
ignore_errors: yes
Copy link
Owner

Choose a reason for hiding this comment

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

could you do failed_when: false instead?

Alternatively, what do you think of moving this task as a lineinfile with check_mode: yes? That might be better?

Copy link
Author

Choose a reason for hiding this comment

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

I think changed_when is appropriate here; the issue is that awk fails, and then no further tasks get executed for that host. Apologies for being a little unclear.

I'll try the lineinfile approach, as it seems like it could be cleaner if it works :)

Copy link
Author

Choose a reason for hiding this comment

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

Nevermind, I was misunderstanding failed_when. Will give that a go also.

Copy link
Owner

Choose a reason for hiding this comment

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

Thanks for the help!

@evrardjp
Copy link
Owner

evrardjp commented Apr 6, 2021

FYI, for context, I didn't use the switch mode for a while.

Maybe it would be good to have this covered by molecule :)

@evrardjp
Copy link
Owner

evrardjp commented Apr 8, 2021

@leigh-thg would you mind giving a look to a PR that rewrites a bunch of this, if I ever find the time? :)

@evrardjp
Copy link
Owner

As you seem to care about this role, I spent some time refactoring it in #59 .
github actions can now test it properly.

It would be great if you could rebase your work on top of this.
As it will be conflicting, just keep the template change (good catch btw!), and I would love to see a new molecule scenario for switch. If you add it into github actions, it's even better, as we can all see it working.

Thank you in advance.

@evrardjp
Copy link
Owner

evrardjp commented Apr 16, 2021

I have updated the role to make it simpler, and increase the test coverage. Can you test if the current role works better for you? If there is something missing, please feel free to update this PR or the one increasing coverage for switch: #69

@leigh-thg leigh-thg closed this Apr 16, 2021
@leigh-thg leigh-thg deleted the master branch April 16, 2021 07:47
@leigh-thg
Copy link
Author

Hi,

Apologies for not getting back to this quicker. I can confirm with #69 that things work as expected.

Many thanks! Leigh

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants