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

Return usable URL from remoteapprest app start #568

Merged
merged 9 commits into from
Sep 7, 2021
33 changes: 28 additions & 5 deletions remoteappmanager/cli/remoteapprest/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import requests
import requests.utils
import json
from urllib.parse import urljoin
from urllib.parse import urljoin, urlsplit

import click

Expand Down Expand Up @@ -137,24 +137,47 @@ def available(ctx):

@app.command()
@click.argument("identifier")
@click.option("--startupdata", default=None)
@click.pass_context
def start(ctx, identifier):
def start(ctx, identifier, startupdata):
"""Starts a container for application identified by IDENTIFIER.
If ``startupdata`` is provided, the container will open the given file at
startup.
If a container is already running, restarts it."""
cred = ctx.obj.credentials
url, username, cookies = cred.url, cred.username, cred.cookies

request_url = urljoin(url,
"/user/{}/api/v1/containers/".format(username))

payload = json.dumps(dict(
mapping_id=identifier
))
payload_dict = dict(mapping_id=identifier)
if startupdata is not None:
# First make sure that the allow_startup_data policy is True
check_url = urljoin(url,
"/user/{}/api/v1/applications/".format(username))
response = requests.get(check_url, cookies=cookies, verify=False)
apps_data = json.loads(response.content.decode("utf-8"))
app_data = apps_data["items"][identifier]
allow_startup_data = app_data["image"]["policy"]["allow_startup_data"]
if allow_startup_data:
payload_dict.update(
configurables=dict(startupdata=dict(startupdata=startupdata)))
else:
raise click.ClickException(
"The 'allow_startup_data' policy is False for the current "
"user.\nExiting.")

payload = json.dumps(payload_dict)

response = requests.post(request_url, payload, cookies=cookies,
verify=False)
if response.status_code == 201:
location = response.headers["Location"]
parsed = urlsplit(location)
path, port = parsed.path.split("_")
port = port.rstrip("/")
path = "".join(path.split("/api/v1"))
location = f"{parsed.scheme}://{parsed.hostname}:{port}{path}"
print(location)
flongford marked this conversation as resolved.
Show resolved Hide resolved


Expand Down
104 changes: 103 additions & 1 deletion remoteappmanager/cli/tests/test_remoteapprest.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,16 +130,118 @@ def test_app_running(self):

def test_app_start(self):
with mock.patch('requests.post') as mock_post, \
mock.patch("requests.get") as mock_get, \
mock.patch("remoteappmanager.cli.remoteapprest.__main__."
"Credentials.from_file") as mock_from_file:

mock_from_file.return_value = self.get_mock_credentials()

self._remoteapprest("app start 1")
response = [mock.Mock()]
response[0].content = json.dumps(
{'items': {'1': {"image": {
"policy": {"allow_startup_data": True}
}}}}
).encode("utf-8")

mock_get.side_effect = response
mock_post.return_value.status_code = 201
mock_post.return_value.headers = {
"Location": "http://127.0.0.1:8000"
"/user/bar/api/v1/containers/test_49165"
}

_, output = self._remoteapprest("app start 1")
self.assertEqual(mock_post.call_args[0][0],
"/user/bar/api/v1/containers/")
self.assertEqual(mock_post.call_args[0][1],
json.dumps({"mapping_id": "1"}))
self.assertEqual(
output.strip(),
"http://127.0.0.1:49165/user/bar/containers/test")

self._remoteapprest("app start 1 --startupdata=/test")
self.assertEqual(mock_post.call_args[0][0],
"/user/bar/api/v1/containers/")
self.assertEqual(
mock_post.call_args[0][1],
json.dumps(
{"mapping_id": "1",
"configurables": {
"startupdata": {"startupdata": "/test"}
}}
))
self.assertEqual(
output.strip(),
"http://127.0.0.1:49165/user/bar/containers/test")

def test_app_start_allow_startup_data(self):
with mock.patch('requests.post') as mock_post, \
mock.patch("requests.get") as mock_get, \
mock.patch("remoteappmanager.cli.remoteapprest.__main__."
"Credentials.from_file") as mock_from_file:

mock_from_file.return_value = self.get_mock_credentials()

# allow_startup_data is False, so this will fail

response = [mock.Mock()]
response[0].content = json.dumps(
{'items': {'1': {"image": {
"policy": {"allow_startup_data": False}
}}}}
).encode("utf-8")

mock_get.side_effect = response
mock_post.return_value.status_code = 201
mock_post.return_value.headers = {
"Location": "http://127.0.0.1:8000"
"/user/bar/api/v1/containers/test_49165"}

argstring = "app start 1 --startupdata=/test"
runner = CliRunner()
result = runner.invoke(remoteapprest.cli,
argstring.split(),
catch_exceptions=True)

self.assertEqual(result.exit_code, 1)
self.assertIn("The 'allow_startup_data' policy is False for the "
"current user",
result.output)

# Removing the --startupdata option should work just fine

argstring = "app start 1"
runner = CliRunner()
result = runner.invoke(remoteapprest.cli,
argstring.split(),
catch_exceptions=True)

self.assertEqual(result.exit_code, 0)
self.assertEqual(
result.output.strip(),
"http://127.0.0.1:49165/user/bar/containers/test")

# Same if the allow_startup_data policy is set to True

response = [mock.Mock()]
response[0].content = json.dumps(
{'items': {'1': {"image": {
"policy": {"allow_startup_data": True}
}}}}
).encode("utf-8")

mock_get.side_effect = response

argstring = "app start 1 --startupdata=/test"
runner = CliRunner()
result = runner.invoke(remoteapprest.cli,
argstring.split(),
catch_exceptions=True)

self.assertEqual(result.exit_code, 0)
self.assertEqual(
result.output.strip(),
"http://127.0.0.1:49165/user/bar/containers/test")

def test_app_stop(self):
with mock.patch('requests.delete') as mock_delete, \
Expand Down
24 changes: 20 additions & 4 deletions remoteappmanager/docker/container_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ def _start_container(self,
'\n'.join('{0} -> {1}'.format(source, target['bind'])
for source, target in filtered_volumes.items()))

container_url_id = _generate_container_url_id()
container_url_id = _generate_container_url_id(image_name)
container_urlpath = without_end_slash(
url_path_join(base_urlpath,
"containers",
Expand Down Expand Up @@ -413,6 +413,8 @@ def _start_container(self,
yield self.stop_and_remove_container(container_id)
raise e

extended_id = f"{container_url_id}_{port}"

container = Container(
docker_id=container_id,
name=container_name,
Expand All @@ -421,7 +423,7 @@ def _start_container(self,
mapping_id=mapping_id,
ip=ip,
port=port,
url_id=container_url_id,
url_id=extended_id,
urlpath=container_urlpath,
)

Expand Down Expand Up @@ -691,8 +693,22 @@ def _generate_container_name(realm, user_name, mapping_id):
)


def _generate_container_url_id():
"""Generates a unique string to identify the container through a url"""
def _generate_container_url_id(image_name=None):
""" If a complete image name is provided, parses and returns the image
name, otherwise generates a unique string to identify the container.

Parameters
----------
image_name : str, default None
The image name, in the format simphony-remote/simphony-application.

Returns
-------
str
The container identifier.
"""
if image_name:
return image_name.split("/")[-1]
return uuid.uuid4().hex


Expand Down