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

support ipv6 for scp with "none" backend #1006

Merged
merged 3 commits into from Apr 20, 2019
Merged

support ipv6 for scp with "none" backend #1006

merged 3 commits into from Apr 20, 2019

Conversation

phryneas
Copy link
Member

Motivation:

Currently, secrets cannot be deployed when deployment.targetHost contains an ipv6.

This is because, while sshing to a ipv6 has the same syntax, scp requires square brackets around the ip. This adds square brackets for calls to scp around any ip with a colon.

I'm no python developer, so please take this PR with a grain of salt. Fortunately, it's not very big ;)

@flokli
Copy link
Contributor

flokli commented Jan 7, 2019

I had this patch applied on top of nixopsUnstable (and it worked deploying when specifying an IPv6 adress), but it seems this breaks copying over secrets:

chekov> copying closure...
network> closures copied successfully
chekov> uploading key ‘foo’...
chekov> error: Traceback (most recent call last):
  File "/nix/store/vjjpmhn8aqmq0vks1h3s3ck69rxqb57g-nixops-1.6.1pre2706_d5ad09c/lib/python2.7/site-packages/nixops/deployment.py", line 721, in worker
    m.send_keys()
  File "/nix/store/vjjpmhn8aqmq0vks1h3s3ck69rxqb57g-nixops-1.6.1pre2706_d5ad09c/lib/python2.7/site-packages/nixops/backends/__init__.py", line 243, in send_keys
    self.upload_file(tmp, tmp_outfile)
  File "/nix/store/vjjpmhn8aqmq0vks1h3s3ck69rxqb57g-nixops-1.6.1pre2706_d5ad09c/lib/python2.7/site-packages/nixops/backends/__init__.py", line 393, in upload_file
    cmdline += [source, "root@" + self.get_ssh_name(True) + ":" + target]
TypeError: get_ssh_name() takes exactly 1 argument (2 given)

Traceback (most recent call last):
  File "/nix/store/vjjpmhn8aqmq0vks1h3s3ck69rxqb57g-nixops-1.6.1pre2706_d5ad09c/bin/..nixops-wrapped-wrapped", line 990, in <module>
    args.op()
  File "/nix/store/vjjpmhn8aqmq0vks1h3s3ck69rxqb57g-nixops-1.6.1pre2706_d5ad09c/bin/..nixops-wrapped-wrapped", line 411, in op_deploy
    max_concurrent_activate=args.max_concurrent_activate)
  File "/nix/store/vjjpmhn8aqmq0vks1h3s3ck69rxqb57g-nixops-1.6.1pre2706_d5ad09c/lib/python2.7/site-packages/nixops/deployment.py", line 1057, in deploy
    self.run_with_notify('deploy', lambda: self._deploy(**kwargs))
  File "/nix/store/vjjpmhn8aqmq0vks1h3s3ck69rxqb57g-nixops-1.6.1pre2706_d5ad09c/lib/python2.7/site-packages/nixops/deployment.py", line 1046, in run_with_notify
    f()
  File "/nix/store/vjjpmhn8aqmq0vks1h3s3ck69rxqb57g-nixops-1.6.1pre2706_d5ad09c/lib/python2.7/site-packages/nixops/deployment.py", line 1057, in <lambda>
    self.run_with_notify('deploy', lambda: self._deploy(**kwargs))
  File "/nix/store/vjjpmhn8aqmq0vks1h3s3ck69rxqb57g-nixops-1.6.1pre2706_d5ad09c/lib/python2.7/site-packages/nixops/deployment.py", line 1013, in _deploy
    dry_activate=dry_activate, max_concurrent_activate=max_concurrent_activate)
  File "/nix/store/vjjpmhn8aqmq0vks1h3s3ck69rxqb57g-nixops-1.6.1pre2706_d5ad09c/lib/python2.7/site-packages/nixops/deployment.py", line 769, in activate_configs
    .format(len(failed), len(res), ", ".join(["‘{0}’".format(x) for x in failed])))
Exception: activation of 1 of 1 machines failed (namely on ‘somehost’)

@phryneas
Copy link
Member Author

phryneas commented Jan 7, 2019

Looks like another get_ssh_name is being called - I didn't account for the other backends. Are you definitely using the ǹone` backend?

@flokli
Copy link
Contributor

flokli commented Jan 7, 2019 via email

@phryneas
Copy link
Member Author

phryneas commented Jan 8, 2019

Ah, okay, I need to update the signature to def get_ssh_name(self, scp=False): in the other providers, too, then.

I'll update this PR in the next days. In the meantime, just update def get_ssh_name(self): in the hetzner provider to def get_ssh_name(self, scp=False):, that should solve it for you for now.

@flokli
Copy link
Contributor

flokli commented Jan 9, 2019

In the meantime, just update def get_ssh_name(self): in the hetzner provider to def get_ssh_name(self, scp=False):, that should solve it for you for now.

Looking at the other implementations of get_ssh_name, shouldn't the [%s] wrapping logic better be placed into a helper function in nixops/backends/__init__.py, used by both upload_file and download_file, instead of passing that ssh parameter through all backends, and applying logic there?

@phryneas
Copy link
Member Author

Yup, I had the same thought. I'm quite busy right now, but I'll try to update this PR in the next weeks. It's not like it hasn't been lying around for a while already ;)

@flokli
Copy link
Contributor

flokli commented Jan 15, 2019

Sure, whenever you find the time :-)

@flokli
Copy link
Contributor

flokli commented Mar 6, 2019

@phryneas - poke, could you update this PR?

@phryneas
Copy link
Member Author

phryneas commented Mar 6, 2019

@flokli oops, sorry. There you go.

@flokli
Copy link
Contributor

flokli commented Mar 6, 2019

LGTM.

CI is red due to the current nixpkgs breakage with azure-mgmt-compute-4.4.0 (unrelated, fixed by NixOS/nixpkgs#56775).

@flokli flokli mentioned this pull request Mar 8, 2019
Open
@AmineChikhaoui AmineChikhaoui merged commit c8d3a3f into NixOS:master Apr 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@phryneas @flokli @AmineChikhaoui