You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by sverrirab <gi...@git.apache.org> on 2016/05/04 13:21:48 UTC
[GitHub] cloudstack pull request: Convert patchviasocket to python (removes...
GitHub user sverrirab opened a pull request:
https://github.com/apache/cloudstack/pull/1533
Convert patchviasocket to python (removes perl dependency for KVM agent)
As requested here: https://github.com/apache/cloudstack/pull/1495
No scripts are using perl so that install requirement can be removed.
The new scripts are using standard python packages only.
Includes extensive unit test.
Note: perl-modules requirement is missing (fixed in mentioned PR) so do not merge that onto master.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/greenqloud/cloudstack pr-patchviasocket-convert-to-python
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/1533.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1533
----
commit 3d8f74f9bc5e6cc3c6170c2320628ce2a2ce532a
Author: Sverrir A. Berg <sv...@greenqloud.com>
Date: 2016-05-03T15:17:59Z
Convert patchviasocket to python (removes perl dependency for KVM agent)
As requested here: https://github.com/apache/cloudstack/pull/1495
No scripts are using perl so that install requirement can be removed.
The new scripts are using standard python packages only.
Includes extensive unit test.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Convert patchviasocket to python (removes...
Posted by sverrirab <gi...@git.apache.org>.
Github user sverrirab commented on the pull request:
https://github.com/apache/cloudstack/pull/1533#issuecomment-220644228
Rebased the PR to latest master and reverted the relevant commit (64b72a5c5a410f41bd869cc9d40807d888e05055.). I think we should be good to go @swill ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Convert patchviasocket to python (removes...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1533#discussion_r62328994
--- Diff: scripts/vm/hypervisor/kvm/test_patchviasocket.py ---
@@ -0,0 +1,142 @@
+#!/usr/bin/env python
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import patchviasocket
+
+import getpass
+import os
+import socket
+import tempfile
+import time
+import threading
+import unittest
+
+KEY_DATA = "I luv\nCloudStack\n"
+CMD_DATA = "/run/this-for-me --please=TRUE! very%quickly"
+NON_EXISTING_FILE = "must-not-exist"
+
+
+def write_key_file():
+ tmpfile = tempfile.mktemp(".sck")
--- End diff --
According to the [Python API document for ``tempfile.mktemp``](https://docs.python.org/2/library/tempfile.html), this method has been deprecated since Python 2.3. ``tempfile.mkstemp`` should be used instead.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Convert patchviasocket to python (removes...
Posted by wido <gi...@git.apache.org>.
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/1533#issuecomment-218761324
Can @sverrirab look at the comments of @jburwell ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Convert patchviasocket to python (removes...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1533#discussion_r62328643
--- Diff: scripts/vm/hypervisor/kvm/test_patchviasocket.py ---
@@ -0,0 +1,142 @@
+#!/usr/bin/env python
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import patchviasocket
+
+import getpass
+import os
+import socket
+import tempfile
+import time
+import threading
+import unittest
+
+KEY_DATA = "I luv\nCloudStack\n"
+CMD_DATA = "/run/this-for-me --please=TRUE! very%quickly"
+NON_EXISTING_FILE = "must-not-exist"
+
+
+def write_key_file():
+ tmpfile = tempfile.mktemp(".sck")
+ with open(tmpfile, "w") as f:
+ f.write(KEY_DATA)
+ return tmpfile
+
+
+class SocketThread(threading.Thread):
+ def __init__(self):
+ super(SocketThread, self).__init__()
+ self._data = ""
+ self._file = tempfile.mktemp(".sck")
+ self._ready = False
+
+ def data(self):
+ return self._data
+
+ def file(self):
+ return self._file
+
+ def wait_until_ready(self):
--- End diff --
Should this be bounded by a maximum wait time? For example, if the ulimits are exceeded, this function could turn into an infinite loop.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Convert patchviasocket to python (removes...
Posted by sverrirab <gi...@git.apache.org>.
Github user sverrirab commented on the pull request:
https://github.com/apache/cloudstack/pull/1533#issuecomment-219751032
@wido I just pushed an update that addresses the comments from @jburwell
tested this locally as well of course
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Convert patchviasocket to python (removes...
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1533#issuecomment-220810862
This is coming back clean. Would you mind trying to rebase and re-push or close and reopen to see if we can kick off travis one more time. Thanks...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Convert patchviasocket to python (removes...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1533#discussion_r62328214
--- Diff: scripts/vm/hypervisor/kvm/patchviasocket.py ---
@@ -0,0 +1,80 @@
+#!/usr/bin/env python
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+#
+# This script connects to the system vm socket and writes the
+# authorized_keys and cmdline data to it. The system VM then
+# reads it from /dev/vport0p1 in cloud_early_config
+#
+
+import argparse
+import os
+import socket
+
+SOCK_FILE = "/var/lib/libvirt/qemu/{name}.agent"
+PUB_KEY_FILE = "/root/.ssh/id_rsa.pub.cloud"
+MESSAGE = "pubkey:{key}\ncmdline:{cmdline}\n"
+
+
+def read_pub_key(key_file):
+ try:
+ if os.path.isfile(key_file):
+ with open(key_file, "r") as f:
+ return f.read()
+ except IOError:
--- End diff --
Would it be helpful to print/log the details of the ``IOError`` for debugging purposes? For example, understanding whether the file was not found or the user lacked sufficient permissions.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Convert patchviasocket to python (removes...
Posted by wido <gi...@git.apache.org>.
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/1533#issuecomment-220583491
Thanks! code-wise this looks good to me now
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Convert patchviasocket to python (removes...
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1533#issuecomment-217566881
@DaanHoogland can you post a 'summary' of the results when you post this. I don't want to have to download each of these files to know that they ran successfully. Can you just say something like "CI Passed" or something like that? "Here are the results" without telling me if it is a success for failure requires me to download and review every file. I understand from your "dont bother with" comment that everything passed except that, but if you can just let me know that status, that will help me. Thanks for testing this. \U0001f44d
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Convert patchviasocket to python (removes...
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1533#issuecomment-217567369
I am missing some code review on this one, but otherwise this is looking good so far.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Convert patchviasocket to python (removes...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1533#discussion_r62328413
--- Diff: scripts/vm/hypervisor/kvm/patchviasocket.py ---
@@ -0,0 +1,80 @@
+#!/usr/bin/env python
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+#
+# This script connects to the system vm socket and writes the
+# authorized_keys and cmdline data to it. The system VM then
+# reads it from /dev/vport0p1 in cloud_early_config
+#
+
+import argparse
+import os
+import socket
+
+SOCK_FILE = "/var/lib/libvirt/qemu/{name}.agent"
+PUB_KEY_FILE = "/root/.ssh/id_rsa.pub.cloud"
+MESSAGE = "pubkey:{key}\ncmdline:{cmdline}\n"
+
+
+def read_pub_key(key_file):
+ try:
+ if os.path.isfile(key_file):
+ with open(key_file, "r") as f:
+ return f.read()
+ except IOError:
+ return None
+
+
+def send_to_socket(sock_file, key_file, cmdline):
+ pub_key = read_pub_key(key_file)
+
+ if not pub_key:
+ print("ERROR: ssh public key not found on host at {0}".format(key_file))
--- End diff --
Why throw this error out of ``read_pub_key`` and consolidate the error handling/return in the exception handler on line 64?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Convert patchviasocket to python (removes...
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1533#issuecomment-220810829
### CI RESULTS
```
Tests Run: 85
Skipped: 0
Failed: 0
Errors: 0
Duration: 8h 55m 29s
```
**Associated Uploads**
**`/tmp/MarvinLogs/DeployDataCenter__May_21_2016_01_21_56_QYQ9XC:`**
* [dc_entries.obj](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1533/tmp/MarvinLogs/DeployDataCenter__May_21_2016_01_21_56_QYQ9XC/dc_entries.obj)
* [failed_plus_exceptions.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1533/tmp/MarvinLogs/DeployDataCenter__May_21_2016_01_21_56_QYQ9XC/failed_plus_exceptions.txt)
* [runinfo.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1533/tmp/MarvinLogs/DeployDataCenter__May_21_2016_01_21_56_QYQ9XC/runinfo.txt)
**`/tmp/MarvinLogs/test_network_T2P1UQ:`**
* [failed_plus_exceptions.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1533/tmp/MarvinLogs/test_network_T2P1UQ/failed_plus_exceptions.txt)
* [results.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1533/tmp/MarvinLogs/test_network_T2P1UQ/results.txt)
* [runinfo.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1533/tmp/MarvinLogs/test_network_T2P1UQ/runinfo.txt)
**`/tmp/MarvinLogs/test_vpc_routers_U3WSE5:`**
* [failed_plus_exceptions.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1533/tmp/MarvinLogs/test_vpc_routers_U3WSE5/failed_plus_exceptions.txt)
* [results.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1533/tmp/MarvinLogs/test_vpc_routers_U3WSE5/results.txt)
* [runinfo.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1533/tmp/MarvinLogs/test_vpc_routers_U3WSE5/runinfo.txt)
Uploads will be available until `2016-07-22 02:00:00 +0200 CEST`
*Comment created by [`upr comment`](https://github.com/cloudops/upr).*
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Convert patchviasocket to python (removes...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1533#discussion_r62329185
--- Diff: scripts/vm/hypervisor/kvm/test_patchviasocket.py ---
@@ -0,0 +1,142 @@
+#!/usr/bin/env python
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import patchviasocket
+
+import getpass
+import os
+import socket
+import tempfile
+import time
+import threading
+import unittest
+
+KEY_DATA = "I luv\nCloudStack\n"
+CMD_DATA = "/run/this-for-me --please=TRUE! very%quickly"
+NON_EXISTING_FILE = "must-not-exist"
+
+
+def write_key_file():
+ tmpfile = tempfile.mktemp(".sck")
+ with open(tmpfile, "w") as f:
+ f.write(KEY_DATA)
+ return tmpfile
+
+
+class SocketThread(threading.Thread):
+ def __init__(self):
+ super(SocketThread, self).__init__()
+ self._data = ""
+ self._file = tempfile.mktemp(".sck")
+ self._ready = False
+
+ def data(self):
+ return self._data
+
+ def file(self):
+ return self._file
+
+ def wait_until_ready(self):
+ while not self._ready:
+ time.sleep(0.050)
+
+ def run(self):
+ TIMEOUT = 0.314 # Very short time for tests that don't write to socket.
+ MAX_SIZE = 10 * 1024
+
+ s = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
+ s.bind(self._file)
+ s.listen(1)
+ s.settimeout(TIMEOUT)
+ try:
+ self._ready = True
+ client, address = s.accept()
+ self._data = client.recv(MAX_SIZE)
+ client.close()
+ except socket.timeout:
+ pass
+ s.close()
+ os.remove(self._file)
--- End diff --
Line 73-74 should be in a ``finally`` block to ensure the resources are properly closed when an exception occurs.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Convert patchviasocket to python (removes...
Posted by sverrirab <gi...@git.apache.org>.
Github user sverrirab commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1533#discussion_r62309904
--- Diff: scripts/vm/hypervisor/kvm/patchviasocket.py ---
@@ -0,0 +1,80 @@
+#!/usr/bin/env python
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+#
+# This script connects to the system vm socket and writes the
+# authorized_keys and cmdline data to it. The system VM then
+# reads it from /dev/vport0p1 in cloud_early_config
+#
+
+import argparse
+import os
+import socket
+
+SOCK_FILE = "/var/lib/libvirt/qemu/{name}.agent"
+PUB_KEY_FILE = "/root/.ssh/id_rsa.pub.cloud"
+MESSAGE = "pubkey:{key}\ncmdline:{cmdline}\n"
+
+
+def read_pub_key(key_file):
+ try:
+ if os.path.isfile(key_file):
+ with open(key_file, "r") as f:
+ return f.read()
+ except IOError:
+ return None
+
+
+def send_to_socket(sock_file, key_file, cmdline):
+ pub_key = read_pub_key(key_file)
+
+ if not pub_key:
+ print("ERROR: ssh public key not found on host at {0}".format(key_file))
+ return 1
+
+ # Keep old substitution from perl code:
+ cmdline = cmdline.replace("%", " ")
+
+ msg = MESSAGE.format(key=pub_key, cmdline=cmdline)
+
+ if not os.path.exists(sock_file):
+ print("ERROR: {0} socket not found".format(sock_file))
+ return 1
+
+ try:
+ s = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
+ s.connect(sock_file)
+ s.sendall(msg)
+ s.close()
+ except IOError as e:
--- End diff --
IOError is the parent of socket.error so this will catch all socket related errors.
The code as it stands returns 1 on any error and 0 on success - so there is no benefit of adding a finally statement there that I can see.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Convert patchviasocket to python (removes...
Posted by jburwell <gi...@git.apache.org>.
Github user jburwell commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1533#discussion_r62328978
--- Diff: scripts/vm/hypervisor/kvm/test_patchviasocket.py ---
@@ -0,0 +1,142 @@
+#!/usr/bin/env python
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import patchviasocket
+
+import getpass
+import os
+import socket
+import tempfile
+import time
+import threading
+import unittest
+
+KEY_DATA = "I luv\nCloudStack\n"
+CMD_DATA = "/run/this-for-me --please=TRUE! very%quickly"
+NON_EXISTING_FILE = "must-not-exist"
+
+
+def write_key_file():
+ tmpfile = tempfile.mktemp(".sck")
+ with open(tmpfile, "w") as f:
+ f.write(KEY_DATA)
+ return tmpfile
+
+
+class SocketThread(threading.Thread):
+ def __init__(self):
+ super(SocketThread, self).__init__()
+ self._data = ""
+ self._file = tempfile.mktemp(".sck")
--- End diff --
According to the [Python API document for ``tempfile.mktemp``](https://docs.python.org/2/library/tempfile.html), this method has been deprecated since Python 2.3. ``tempfile.mkstemp`` should be used instead.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Convert patchviasocket to python (removes...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cloudstack/pull/1533
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Convert patchviasocket to python (removes...
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1533#issuecomment-220633650
@sverrirab thank you. Would you mind adding the revert of #1495 into this commit in order to clean up unnecessary packages once this PR is accepted? I think we are missing one code review on this one and it is pretty much ready to go. Thanks...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Convert patchviasocket to python (removes...
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1533#issuecomment-221162872
Yes, ready now. Thank you. :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Convert patchviasocket to python (removes...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1533#issuecomment-217560805
I hadn't looked into the code yet so for what it's worth and pending response to the comments:
CI RESULTS
[1533.results.internal_lb.txt](https://github.com/apache/cloudstack/files/253034/1533.results.internal_lb.txt)
[1533.results.network.txt](https://github.com/apache/cloudstack/files/253035/1533.results.network.txt)
[1533.results.vpc_routers.txt](https://github.com/apache/cloudstack/files/253037/1533.results.vpc_routers.txt)
[1533.results.vpc_vpn.txt](https://github.com/apache/cloudstack/files/253036/1533.results.vpc_vpn.txt)
I didn't bother with the known ping problem in test_02_redundant_VPC_default_routes
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Convert patchviasocket to python (removes...
Posted by sverrirab <gi...@git.apache.org>.
Github user sverrirab commented on the pull request:
https://github.com/apache/cloudstack/pull/1533#issuecomment-221026255
everything looking good now it seems - time to merge @swill ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Convert patchviasocket to python (removes...
Posted by wido <gi...@git.apache.org>.
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/1533#issuecomment-217011034
Thanks for this! In the future I'd like to see this done through the Qemu Guest Agent, but that is still waiting for something, see: #985
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Convert patchviasocket to python (removes...
Posted by wido <gi...@git.apache.org>.
Github user wido commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1533#discussion_r62118856
--- Diff: scripts/vm/hypervisor/kvm/patchviasocket.py ---
@@ -0,0 +1,80 @@
+#!/usr/bin/env python
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+#
+# This script connects to the system vm socket and writes the
+# authorized_keys and cmdline data to it. The system VM then
+# reads it from /dev/vport0p1 in cloud_early_config
+#
+
+import argparse
+import os
+import socket
+
+SOCK_FILE = "/var/lib/libvirt/qemu/{name}.agent"
+PUB_KEY_FILE = "/root/.ssh/id_rsa.pub.cloud"
+MESSAGE = "pubkey:{key}\ncmdline:{cmdline}\n"
+
+
+def read_pub_key(key_file):
+ try:
+ if os.path.isfile(key_file):
+ with open(key_file, "r") as f:
+ return f.read()
+ except IOError:
+ return None
+
+
+def send_to_socket(sock_file, key_file, cmdline):
+ pub_key = read_pub_key(key_file)
+
+ if not pub_key:
+ print("ERROR: ssh public key not found on host at {0}".format(key_file))
+ return 1
+
+ # Keep old substitution from perl code:
+ cmdline = cmdline.replace("%", " ")
+
+ msg = MESSAGE.format(key=pub_key, cmdline=cmdline)
+
+ if not os.path.exists(sock_file):
+ print("ERROR: {0} socket not found".format(sock_file))
+ return 1
+
+ try:
+ s = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
+ s.connect(sock_file)
+ s.sendall(msg)
+ s.close()
+ except IOError as e:
--- End diff --
Is IOError the only exception which can occur here?
Wouldn't it be safer to but a 'return 1' in a 'finally' statement and otherwise return 0?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: Convert patchviasocket to python (removes...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1533#issuecomment-217569401
Sorry Will, Yes all passed, some after retest.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---