Merge lp:~canonical-platform-qa/snappy-ecosystem-tests/fix-snapd-staging into lp:snappy-ecosystem-tests

[Omer Akram]

Enable snapd staging environment

Enable snapd staging environment.

Note: Currently its downloading from my personal ppa, we will update that soon once we have new PPAs created and get in place a mechanism to upload automatically to ~canonical-platform-qa staging ppas.

[lp:~canonical-platform-qa/snappy-ecosystem-tests/fix-snapd-staging]

51. By Omer Akram on 2017-03-09

merge with trunk

[platform-qa-bot (platform-qa-bot)]

FAILED: Continuous integration, rev:50
https://platform-qa-jenkins.ubuntu.com/job/snappy-ecosystem-tests-ci/270/
Executed test runs:
    None: https://platform-qa-jenkins.ubuntu.com/job/generic-update-mp/2092/console

Click here to trigger a rebuild:
https://platform-qa-jenkins.ubuntu.com/job/snappy-ecosystem-tests-ci/270/rebuild

review: Needs Fixing (continuous-integration)

[Omer Akram (om26er)]

Note: snapd staging environment does not require those environment variables that I removed. Also to allow for installation from ppa I had to remove "xenial" as default channel.

Also note: the code is now executed inside sh -c because it allows us to run complex commands like one line conditions.

[platform-qa-bot (platform-qa-bot)]

PASSED: Continuous integration, rev:51
https://platform-qa-jenkins.ubuntu.com/job/snappy-ecosystem-tests-ci/271/
Executed test runs:
    None: https://platform-qa-jenkins.ubuntu.com/job/generic-update-mp/2093/console

Click here to trigger a rebuild:
https://platform-qa-jenkins.ubuntu.com/job/snappy-ecosystem-tests-ci/271/rebuild

review: Approve (continuous-integration)

[Santiago Baldassin (sbaldassin)]

Code looks good to me

review: Approve

[lp:~canonical-platform-qa/snappy-ecosystem-tests/fix-snapd-staging]

52. By Omer Akram on 2017-03-09

merge with trunk

[platform-qa-bot (platform-qa-bot)]

PASSED: Continuous integration, rev:52
https://platform-qa-jenkins.ubuntu.com/job/snappy-ecosystem-tests-ci/275/
Executed test runs:
    None: https://platform-qa-jenkins.ubuntu.com/job/generic-update-mp/2104/console

Click here to trigger a rebuild:
https://platform-qa-jenkins.ubuntu.com/job/snappy-ecosystem-tests-ci/275/rebuild

review: Approve (continuous-integration)

[I Ahmad (iahmad)]

inline diff

[Omer Akram (om26er)]

Added a comment inline.

[lp:~canonical-platform-qa/snappy-ecosystem-tests/fix-snapd-staging]

53. By Omer Akram on 2017-03-10

merge with trunk

[platform-qa-bot (platform-qa-bot)]

PASSED: Continuous integration, rev:53
https://platform-qa-jenkins.ubuntu.com/job/snappy-ecosystem-tests-ci/282/
Executed test runs:
    None: https://platform-qa-jenkins.ubuntu.com/job/generic-update-mp/2111/console

Click here to trigger a rebuild:
https://platform-qa-jenkins.ubuntu.com/job/snappy-ecosystem-tests-ci/282/rebuild

review: Approve (continuous-integration)

[lp:~canonical-platform-qa/snappy-ecosystem-tests/fix-snapd-staging]

54. By Omer Akram on 2017-03-10

merge with trunk

55. By Omer Akram on 2017-03-10

merge with trunk

[platform-qa-bot (platform-qa-bot)]

PASSED: Continuous integration, rev:55
https://platform-qa-jenkins.ubuntu.com/job/snappy-ecosystem-tests-ci/283/
Executed test runs:
    None: https://platform-qa-jenkins.ubuntu.com/job/generic-update-mp/2112/console

Click here to trigger a rebuild:
https://platform-qa-jenkins.ubuntu.com/job/snappy-ecosystem-tests-ci/283/rebuild

review: Approve (continuous-integration)

[Omer Akram (om26er)]

One problem with this branch is that its setting permanent environment variables to containers inside /etc/environment. While that is fine for snapd given it reads from there only but for snapcraft we need another solution here.

How about adding methods to Snapcraft and Snapd classes to toggle staging/production/XX urls inside the container. This will also help us to remove the need to pass in environment variables through ssh commands.

[lp:~canonical-platform-qa/snappy-ecosystem-tests/fix-snapd-staging]

56. By Omer Akram on 2017-03-10

fix staging search url

[platform-qa-bot (platform-qa-bot)]

PASSED: Continuous integration, rev:56
https://platform-qa-jenkins.ubuntu.com/job/snappy-ecosystem-tests-ci/285/
Executed test runs:
    None: https://platform-qa-jenkins.ubuntu.com/job/generic-update-mp/2114/console

Click here to trigger a rebuild:
https://platform-qa-jenkins.ubuntu.com/job/snappy-ecosystem-tests-ci/285/rebuild

review: Approve (continuous-integration)

[lp:~canonical-platform-qa/snappy-ecosystem-tests/fix-snapd-staging]

57. By Omer Akram on 2017-03-10

Switch store on demand

[platform-qa-bot (platform-qa-bot)]

FAILED: Continuous integration, rev:57
https://platform-qa-jenkins.ubuntu.com/job/snappy-ecosystem-tests-ci/286/
Executed test runs:
    None: https://platform-qa-jenkins.ubuntu.com/job/generic-update-mp/2115/console

Click here to trigger a rebuild:
https://platform-qa-jenkins.ubuntu.com/job/snappy-ecosystem-tests-ci/286/rebuild

review: Needs Fixing (continuous-integration)

[lp:~canonical-platform-qa/snappy-ecosystem-tests/fix-snapd-staging]

58. By Omer Akram on 2017-03-10

add production urls

[platform-qa-bot (platform-qa-bot)]

FAILED: Continuous integration, rev:58
https://platform-qa-jenkins.ubuntu.com/job/snappy-ecosystem-tests-ci/287/
Executed test runs:
    None: https://platform-qa-jenkins.ubuntu.com/job/generic-update-mp/2116/console

Click here to trigger a rebuild:
https://platform-qa-jenkins.ubuntu.com/job/snappy-ecosystem-tests-ci/287/rebuild

review: Needs Fixing (continuous-integration)

[lp:~canonical-platform-qa/snappy-ecosystem-tests/fix-snapd-staging]

59. By Omer Akram on 2017-03-10

Make pylint happy by reducing code duplication

60. By Omer Akram on 2017-03-10

Lets not export environment variables for each ssh command

[platform-qa-bot (platform-qa-bot)]

PASSED: Continuous integration, rev:60
https://platform-qa-jenkins.ubuntu.com/job/snappy-ecosystem-tests-ci/288/
Executed test runs:
    None: https://platform-qa-jenkins.ubuntu.com/job/generic-update-mp/2117/console

Click here to trigger a rebuild:
https://platform-qa-jenkins.ubuntu.com/job/snappy-ecosystem-tests-ci/288/rebuild

review: Approve (continuous-integration)

[lp:~canonical-platform-qa/snappy-ecosystem-tests/fix-snapd-staging]

61. By Omer Akram on 2017-03-10

remove environment variable whitelist code as that's obsolete with the new approach

62. By Omer Akram on 2017-03-10

more removal of unneeded code

[platform-qa-bot (platform-qa-bot)]

PASSED: Continuous integration, rev:61
https://platform-qa-jenkins.ubuntu.com/job/snappy-ecosystem-tests-ci/289/
Executed test runs:
    None: https://platform-qa-jenkins.ubuntu.com/job/generic-update-mp/2118/console

Click here to trigger a rebuild:
https://platform-qa-jenkins.ubuntu.com/job/snappy-ecosystem-tests-ci/289/rebuild

review: Approve (continuous-integration)

[platform-qa-bot (platform-qa-bot)]

PASSED: Continuous integration, rev:62
https://platform-qa-jenkins.ubuntu.com/job/snappy-ecosystem-tests-ci/290/
Executed test runs:
    None: https://platform-qa-jenkins.ubuntu.com/job/generic-update-mp/2119/console

Click here to trigger a rebuild:
https://platform-qa-jenkins.ubuntu.com/job/snappy-ecosystem-tests-ci/290/rebuild

review: Approve (continuous-integration)

[Omer Akram (om26er)]

Updated this branch to add new methods to Snapcraft and Snapd to dynamically switch the store they are interested in. This also allowed me to remove code that needed to whitelist variable exports over ssh.

So basically a test, self.snapd.switch_store('production') and it will point to production servers, similar snapcraft can do the toggle as well.

One open question: Do we still externally want to decide if a test should be run in production or staging through environment variable ? Or is it fine to give that responsibility to the test. OR do we want both ?

Our base test class could read from an environment variable if it should direct to staging or production for external control.

Thoughts ?

[Heber Parrucci (heber013)]

> Updated this branch to add new methods to Snapcraft and Snapd to dynamically
> switch the store they are interested in. This also allowed me to remove code
> that needed to whitelist variable exports over ssh.
>
> So basically a test, self.snapd.switch_store('production') and it will point
> to production servers, similar snapcraft can do the toggle as well.
>
> One open question: Do we still externally want to decide if a test should be
> run in production or staging through environment variable ? Or is it fine to
> give that responsibility to the test. OR do we want both ?
>
> Our base test class could read from an environment variable if it should
> direct to staging or production for external control.
>
> Thoughts ?

IMHO we want to point the environment to a given store: prod, staging, etc. But this should be resolved in environment setup. The tests shouldn't know if they are being executed against a particular store. What we might want is pointing different product to different stores, for example:
snapd --> prod
snapcraft --> staging

But this should be resolved in setup time. I think it this dangerous if we give to the tests the capability to switch to another store. Because we will end up not being sure what environment we are actually testing.

[Heber Parrucci (heber013)]

Comment inline

review: Needs Fixing

[lp:~canonical-platform-qa/snappy-ecosystem-tests/fix-snapd-staging]

63. By Omer Akram on 2017-03-10

Add FIXME to replace PPAs with one from team

[platform-qa-bot (platform-qa-bot)]

PASSED: Continuous integration, rev:63
https://platform-qa-jenkins.ubuntu.com/job/snappy-ecosystem-tests-ci/292/
Executed test runs:
    None: https://platform-qa-jenkins.ubuntu.com/job/generic-update-mp/2121/console

Click here to trigger a rebuild:
https://platform-qa-jenkins.ubuntu.com/job/snappy-ecosystem-tests-ci/292/rebuild

review: Approve (continuous-integration)

[Omer Akram (om26er)]

replied inline.

[Omer Akram (om26er)]

Heber, added a reply inline.

What this code is doing is, it is setting up staging environment OR production environment based on what parameter was provided to run_setup script. Now we no longer need to pass in environment variables over ssh as they are put inside the container during setup.

The switch_store() methods that I have written allow for dynamic switching by the test code, if this is not what we desire I can remove those functions. But we would lose the flexibility of changing/toggling between staging/production from within our test and the environment we first setup with run_setup is going to be permanent.

I would like to know what others think about that.

[Heber Parrucci (heber013)]

> Heber, added a reply inline.
>
> What this code is doing is, it is setting up staging environment OR production
> environment based on what parameter was provided to run_setup script. Now we
> no longer need to pass in environment variables over ssh as they are put
> inside the container during setup.
>
> The switch_store() methods that I have written allow for dynamic switching by
> the test code, if this is not what we desire I can remove those functions. But
> we would lose the flexibility of changing/toggling between staging/production
> from within our test and the environment we first setup with run_setup is
> going to be permanent.
>
> I would like to know what others think about that.

The question would be: are there any case in which we will want to switch the store inside a tests execution?
In my opinion the setup prepares the env pointing to one store (or 2 of we want to point snapd and snapcraft to different stores) and this is permanent in all the tests execution, so you know what you are testing. If you dynamically allow the tests to switch the store, you will not be sure if the tests passed for: prod, staging, or whatever env you want to test.

[Omer Akram (om26er)]

We can dynamically switch stores and be *sure* about that in our test code, (ensure it in setUp() lets say). Though if there isn't any use case, removing does make sense.

[I Ahmad (iahmad)]

I agree with Heber, don't see any use case for current scope of testing. We can always add it if needed in future.

[lp:~canonical-platform-qa/snappy-ecosystem-tests/fix-snapd-staging]

64. By Omer Akram on 2017-03-13

remove dynamic environment switchers

[platform-qa-bot (platform-qa-bot)]

PASSED: Continuous integration, rev:64
https://platform-qa-jenkins.ubuntu.com/job/snappy-ecosystem-tests-ci/293/
Executed test runs:
    None: https://platform-qa-jenkins.ubuntu.com/job/generic-update-mp/2128/console

Click here to trigger a rebuild:
https://platform-qa-jenkins.ubuntu.com/job/snappy-ecosystem-tests-ci/293/rebuild

review: Approve (continuous-integration)

[Omer Akram (om26er)]

Lets land this.

[Heber Parrucci (heber013)]

Code LGTM and it works properly.

review: Approve

=== modified file 'snappy_ecosystem_tests/environment/constants.py'
--- snappy_ecosystem_tests/environment/constants.py	2017-03-10 10:18:26 +0000
+++ snappy_ecosystem_tests/environment/constants.py	2017-03-13 08:59:44 +0000
@@ -32,18 +32,13 @@
 PROFILES = {
     "staging": {
         SNAPD: {
-            "channel": "xenial",
+            "channel": "",
             "package_name": 'snapd',
             "environment_variables": {
-                "UBUNTU_STORE_API_ROOT_URL":
-                    "https://myapps.developer.ubuntu.com/dev/api/",
-                "UBUNTU_STORE_UPLOAD_ROOT_URL":
-                    "https://upload.apps.ubuntu.com",
-                "UBUNTU_SSO_API_ROOT_URL":
-                    "https://login.ubuntu.com/api/v2/",
-                "UBUNTU_STORE_SEARCH_ROOT_URL":
-                    "https://search.apps.ubuntu.com/"
-            }
+                "SNAPPY_USE_STAGING_STORE": "1"
+            },
+            # FIXME: Update ppa to ~canonical-platform-qa team once ready.
+            "ppas": ["ppa:om26er/snapd-candidate"]
         },
         SNAPCRAFT: {
             "channel": "xenial",
@@ -57,7 +52,37 @@
                     "https://login.staging.ubuntu.com/api/v2/",
                 "UBUNTU_STORE_SEARCH_ROOT_URL":
                     "https://search.apps.staging.ubuntu.com/"
-            }
+            },
+            "ppas": []
+        },
+        "snapweb": {
+            "url": "https://myapps.developer.staging.ubuntu.com/"
+        },
+    },
+    "production": {
+        SNAPD: {
+            "channel": "",
+            "package_name": 'snapd',
+            "environment_variables": {
+                "SNAPPY_USE_STAGING_STORE": "0"
+            },
+            # FIXME: Update ppa to ~canonical-platform-qa team once ready.
+            "ppas": ["ppa:om26er/snapd-candidate"]
+        },
+        SNAPCRAFT: {
+            "channel": "xenial",
+            "package_name": 'snapcraft',
+            "environment_variables": {
+                "UBUNTU_STORE_API_ROOT_URL":
+                    "https://myapps.developer.ubuntu.com/dev/api/",
+                "UBUNTU_STORE_UPLOAD_ROOT_URL":
+                    "https://upload.apps.ubuntu.com/",
+                "UBUNTU_SSO_API_ROOT_URL":
+                    "https://login.ubuntu.com/api/v2/",
+                "UBUNTU_STORE_SEARCH_ROOT_URL":
+                    "https://search.apps.ubuntu.com/"
+            },
+            "ppas": []
         },
         "snapweb": {
             "url": "https://myapps.developer.ubuntu.com/"

=== modified file 'snappy_ecosystem_tests/environment/containers/lxd.py'
--- snappy_ecosystem_tests/environment/containers/lxd.py	2017-03-10 09:32:09 +0000
+++ snappy_ecosystem_tests/environment/containers/lxd.py	2017-03-13 08:59:44 +0000
@@ -31,9 +31,6 @@
 from snappy_ecosystem_tests.environment.data.snapd import (
     DEFAULT_CONTAINER_CONFIG)
 
-COMMAND_ALLOW_ENV_VARS = 'sed -i \'/^AcceptEnv/ s/$/ {}/\' ' \
-                         '/etc/ssh/sshd_config'
-
 LOGGER = logging.getLogger(__name__)
 DOMAIN_PING = 'ubuntu.com'
 
@@ -73,6 +70,7 @@
         :return: stdout of the command.
         :raises ValueError: If command exists with a non-zero code.
         """
+        command_string = 'sh -c \'{}\''.format(command_string)
         response = container.execute(shlex.split(command_string))
         if response.exit_code == 0:
             LOGGER.info(response.stdout.strip())
@@ -86,15 +84,40 @@
             if not cont.state().status == 'Running':
                 cont.start(wait=True)
 
-    @staticmethod
-    def install(container, packages, channel=None):
+    def install(self, container, packages, channel=None):
         """Install packages"""
-        LXDDriver._execute_command(container, 'apt-get update')
-        install_command = 'apt-get install {} -y'.format(
+        self._execute_command(container, 'apt update')
+        install_command = 'apt install {} -y --allow-downgrades'.format(
             ' '.join(packages))
         if channel:
             install_command += ' -t {}'.format(channel)
-        LXDDriver._execute_command(container, install_command)
+        self._execute_command(container, install_command)
+
+    def add_ppas(self, container, ppas, pin_priority=1001):
+        """Add the given PPAs inside the container and set their pin priority.
+        """
+        for ppa in ppas:
+            ppa_name = '-'.join(ppa.split(':')[1].split('/'))
+            self._execute_command(
+                container,
+                'add-apt-repository {} -y'.format(ppa))
+            pin_config = '/etc/apt/preferences.d/{}'.format(ppa_name)
+            self._execute_command(
+                container,
+                'if [ -f {conf_file} ]; then rm {conf_file}; fi'.format(
+                    conf_file=pin_config))
+            self._execute_command(
+                container,
+                'echo "Package: *" >> {}'.format(pin_config))
+            self._execute_command(
+                container,
+                'echo "Pin: release o=LP-PPA-{}" >> {}'.format(
+                    ppa_name, pin_config))
+            self._execute_command(
+                container,
+                'echo "Pin-Priority: {}" >> {}'.format(
+                    pin_priority, pin_config))
+        self._execute_command(container, 'apt update')
 
     @staticmethod
     @retry(stop_max_attempt_number=5, wait_fixed=3000,
@@ -118,11 +141,11 @@
                 container.name))
 
     def export_environment_variables(self, container, environment_variables):
-        """Allow export of environment variables over ssh."""
-        self._execute_command(
-            container,
-            COMMAND_ALLOW_ENV_VARS.format(' '.join(environment_variables)))
-        self._execute_command(container, 'service ssh restart')
+        """Export permanent environment variables inside container."""
+        for key, value in environment_variables.items():
+            self._execute_command(
+                container,
+                'echo \'{}={}\' >> /etc/environment'.format(key, value))
 
     def wait_for_internet(self, container, attempts_count=10,
                           attempt_interval=1, ping_domain=DOMAIN_PING):

=== modified file 'snappy_ecosystem_tests/environment/managers.py'
--- snappy_ecosystem_tests/environment/managers.py	2017-03-09 10:41:13 +0000
+++ snappy_ecosystem_tests/environment/managers.py	2017-03-13 08:59:44 +0000
@@ -44,6 +44,12 @@
         containers = self.driver.launch(CONTAINERS)
         for cont in containers:
             self.driver.wait_for_internet(cont)
+            self.driver.export_environment_variables(
+                cont,
+                PROFILES[profile][cont.name]["environment_variables"])
+            self.driver.add_ppas(
+                cont,
+                PROFILES[profile][cont.name]["ppas"])
             deps = DEPENDENCIES[cont.name] + DEPENDENCIES["shared"]
             self.driver.install(cont, packages=deps)
             self.driver.install(
@@ -51,6 +57,3 @@
                 packages=[PROFILES[profile][cont.name]["package_name"]],
                 channel=PROFILES[profile][cont.name]["channel"])
             self.driver.enable_ssh(cont)
-            self.driver.export_environment_variables(
-                cont,
-                PROFILES[profile][cont.name]["environment_variables"].keys())

=== modified file 'snappy_ecosystem_tests/helpers/snapcraft/client.py'
--- snappy_ecosystem_tests/helpers/snapcraft/client.py	2017-03-10 10:28:15 +0000
+++ snappy_ecosystem_tests/helpers/snapcraft/client.py	2017-03-13 08:59:44 +0000
@@ -24,7 +24,6 @@
 import subprocess
 import time
 
-from snappy_ecosystem_tests.environment.constants import PROFILES, SNAPCRAFT
 from snappy_ecosystem_tests.models.snap import Snap
 from snappy_ecosystem_tests.utils.commands import build_command
 from snappy_ecosystem_tests.utils.filters import filter_list
@@ -88,10 +87,7 @@
             command = build_snapcraft_command(command)
         if debug:
             command += ' -d'
-        return self.ssh.exec_command(
-            command,
-            environment=PROFILES[get_current_store()][SNAPCRAFT]
-            ["environment_variables"])
+        return self.ssh.exec_command(command)
 
     def assert_logged_in(self):
         """Assert that an user is logged.
@@ -105,9 +101,7 @@
         """Return bool representing if the user is logged into snapcraft."""
         try:
             output = self.ssh.exec_command(
-                'cat %s' % SNAPCRAFT_CONFIG_FILE_PATH,
-                environment=PROFILES[get_current_store()][SNAPCRAFT]
-                ["environment_variables"])
+                'cat %s' % SNAPCRAFT_CONFIG_FILE_PATH)
             return output is not ''
         except ValueError:
             return False
@@ -131,9 +125,7 @@
         """
         self.ssh.exec_command(
             COMMANDS_LOGIN.format(email=email, password=password,
-                                  snapcraft=PATH_SNAPCRAFT),
-            environment=PROFILES[get_current_store()][SNAPCRAFT]
-            ["environment_variables"])
+                                  snapcraft=PATH_SNAPCRAFT))
         return self.is_logged_in()
 
     def list_registered(self, debug=True, raw=False):