#38 CLI support for building modules
Merged 7 years ago by clime. Opened 7 years ago by frostyx.
copr/ frostyx/copr mbs-submit  into  master

@@ -0,0 +1,105 @@ 

+ #!/bin/bash

+ # vim: dict=/usr/share/beakerlib/dictionary.vim cpt=.,w,b,u,t,i,k

+ # ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

+ #

+ #   runtest-modules.sh of /tools/copr/Sanity/copr-cli-basic-operations

+ #   Description: Tests basic operations of copr using copr-cli.

+ #   Author: Jakub Kadlcik <jkadlcik@redhat.com>

+ #

+ # ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

+ #

+ #   Copyright (c) 2014 Red Hat, Inc.

+ #

+ #   This program is free software: you can redistribute it and/or

+ #   modify it under the terms of the GNU General Public License as

+ #   published by the Free Software Foundation, either version 2 of

+ #   the License, or (at your option) any later version.

+ #

+ #   This program is distributed in the hope that it will be

+ #   useful, but WITHOUT ANY WARRANTY; without even the implied

+ #   warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR

+ #   PURPOSE.  See the GNU General Public License for more details.

+ #

+ #   You should have received a copy of the GNU General Public License

+ #   along with this program. If not, see http://www.gnu.org/licenses/.

+ #

+ # ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

+ 

+ # Include Beaker environment

+ . /usr/bin/rhts-environment.sh || exit 1

+ . /usr/share/beakerlib/beakerlib.sh || exit 1

+ 

+ if [[ ! $FRONTEND_URL ]]; then

+     FRONTEND_URL="http://copr-fe-dev.cloud.fedoraproject.org"

+ fi

+ if [[ ! $BACKEND_URL ]]; then

+     BACKEND_URL="http://copr-be-dev.cloud.fedoraproject.org"

+ fi

+ 

+ echo "FRONTEND_URL = $FRONTEND_URL"

+ echo "BACKEND_URL = $BACKEND_URL"

+ 

+ SCRIPT=`realpath $0`

+ HERE=`dirname $SCRIPT`

+ 

+ rlJournalStart

+     rlPhaseStartSetup

+         rlAssertRpm "copr-cli"

+         rlAssertExists ~/.config/copr

+         # testing instance?

+         rlAssertGrep "$FRONTEND_URL" ~/.config/copr

+         # we don't need to be destroying the production instance

+         rlAssertNotGrep "copr.fedoraproject.org" ~/.config/copr

+         # token ok? communication ok?

+         rlRun "copr-cli list"

+         # and install... things

+         yum -y install dnf dnf-plugins-core

+         # use the dev instance

+         sed -i "s+http://copr.fedoraproject.org+$FRONTEND_URL+g" \

+         /usr/lib/python3.4/site-packages/dnf-plugins/copr.py

+         sed -i "s+https://copr.fedoraproject.org+$FRONTEND_URL+g" \

+         /usr/lib/python3.4/site-packages/dnf-plugins/copr.py

+         dnf -y install jq

+     rlPhaseEnd

+ 

+     rlPhaseStartTest

+ 

+         # Test yaml submit

+         DATE=$(date +%s)

+         echo "version=$DATE"

+         yes | cp $HERE/files/testmodule.yaml /tmp

+         sed -i "s/\$VERSION/$DATE/g" /tmp/testmodule.yaml

+         rlRun "copr-cli build-module --yaml /tmp/testmodule.yaml"

+ 

+         # Test module duplicity

+         # @FIXME the request sometimes hangs for some obscure reason

+         OUTPUT=`mktemp`

+         rlRun "copr-cli build-module --yaml /tmp/testmodule.yaml &> $OUTPUT" 1

+         rlAssertEquals "Module should already exist" `cat $OUTPUT | grep "already exists" |wc -l` 1

+ 

+         # @TODO Test scmurl submit

+         # We can't exactly say whether such NSV was built yet so we don't

+         # know whether to anticipate a success or an duplicity eror.

+         # The whole idea of modules duplicity should be resolved after

+         # some time of using the MBS. See a related RFE

+         # https://pagure.io/fm-orchestrator/issue/308

+ 

+         # @TODO Test that MBS api is not accessible

+         # Not yet configured

+ 

+         # @TODO Test that module succeeded

+         # We should wait in loop (with some timeout)

+         # and check builds and module state

+ 

+         # @TODO Test that module can be enabled with dnf

+         # Feature for enabling module from Copr is not in upstream

+ 

+         # @TODO Test that enabled module info is correct

+         # Feature for enabling module from Copr is not in upstream

+ 

+     rlPhaseEnd

+ 

+     rlPhaseStartCleanup

+     rlPhaseEnd

+ rlJournalPrintText

+ rlJournalEnd

file modified
+69 -2
@@ -10,13 +10,14 @@ 

  import time

  import six

  import simplejson

+ import random

  from collections import defaultdict

  

  import logging

  if six.PY2:

-     from urlparse import urlparse

+     from urlparse import urlparse, urlencode

  else:

-     from urllib.parse import urlparse

+     from urllib.parse import urlparse, urlencode

  

  if sys.version_info < (2, 7):

      class NullHandler(logging.Handler):
@@ -32,6 +33,7 @@ 

  import copr.exceptions as copr_exceptions

  

  from .util import ProgressBar

+ from .util import listen_for_token

  from .build_config import MockProfile

  

  
@@ -559,6 +561,55 @@ 

          if not args.nowait:

              self._watch_builds(build_ids)

  

+     def action_build_module(self, args):

+         """

+         Build module via Copr MBS

+         """

+         token = args.token

+         if not token:

+             print("Provide token as command line argument or visit following URL to obtain the token:")

+             query = urlencode({

+                 'response_type': 'token',

+                 'response_mode': 'form_post',

+                 'nonce': random.randint(100, 10000),

+                 'scope': ' '.join([

+                     'openid',

+                     'https://id.fedoraproject.org/scope/groups',

+                     'https://mbs.fedoraproject.org/oidc/submit-build',

+                 ]),

+                 'client_id': 'mbs-authorizer',

+             }) + "&redirect_uri=http://localhost:13747/"

+             print("https://id.fedoraproject.org/openidc/Authorization?" + query)

+             print("We are waiting for you to finish the token generation...")

+ 

+             token = listen_for_token()

+             print("Token: {}".format(token))

+             print("")

+         if not token:

+             print("Failed to get a token from response")

+             sys.exit(1)

+ 

+         modulemd = open(args.yaml, "rb") if args.yaml else args.url

+         response = self.client.build_module(modulemd, token)

+         if response.status_code == 500:

+             print(response.text)

+             sys.exit(1)

+ 

+         data = response.json()

+         if response.status_code != 201:

+             print("Error: {}".format(data["message"]))

+             sys.exit(1)

+         print("Created module {}-{}-{}".format(data["name"], data["stream"], data["version"]))

+ 

+     def action_make_module(self, args):

+         """

+         Fake module build

+         """

+         ownername, projectname = parse_name(args.copr)

+         result = self.client.make_module(projectname, args.yaml, username=ownername)

+         print(result.message if result.output == "ok" else result.error)

+ 

+ 

  def setup_parser():

      """

      Set the main arguments.
@@ -941,6 +992,22 @@ 

                                        metavar="PKGNAME", required=True)

      parser_build_package.set_defaults(func="action_build_package")

  

+     # module building

+     parser_build_module = subparsers.add_parser("build-module", help="Builds a given module in Copr")

+     parser_build_module_mmd_source = parser_build_module.add_mutually_exclusive_group()

+     parser_build_module_mmd_source.add_argument("--url", help="SCM with modulemd file in yaml format")

+     parser_build_module_mmd_source.add_argument("--yaml", help="Path to modulemd file in yaml format")

+     parser_build_module.add_argument("--token",

+                                      help="OIDC token for module build service")

+     parser_build_module.set_defaults(func="action_build_module")

+ 

+     parser_make_module = subparsers.add_parser("make-module", help="Makes a module in Copr")

+     parser_make_module.add_argument("copr",

+                                     help="The copr repo to build the module in. Can be just name of project or even in format username/project or @groupname/project.")

+     parser_make_module.add_argument("--yaml",

+                                     help="Path to modulemd file in yaml format")

+     parser_make_module.set_defaults(func="action_make_module")

+ 

      return parser

  

  

file modified
+57
@@ -1,5 +1,8 @@ 

  # coding: utf-8

  

+ import socket

+ from datetime import timedelta

+ 

  try:

      from progress.bar import Bar

  except ImportError:
@@ -50,3 +53,57 @@ 

          suffix = "%(downloaded)s %(download_speed)s eta %(eta_td)s"

  else:

      ProgressBar = DummyBar

+ 

+ 

+ def listen_for_token():

+     """

+     Function taken from https://pagure.io/fm-orchestrator/blob/master/f/contrib/submit_build.py

+     We should avoid code duplicity by including it into _some_ python module

+ 

+     Listens on port 13747 on localhost for a redirect request by OIDC

+     server, parses the response and returns the "access_token" value.

+     """

+     TCP_IP = '0.0.0.0'

+     TCP_PORT = 13747

+     BUFFER_SIZE = 1024

+ 

+     s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)

+     s.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)

+     s.bind((TCP_IP, TCP_PORT))

+     s.listen(1)

+ 

+     conn, addr = s.accept()

+     data = ""

+     sent_resp = False

+     while 1:

+         try:

+             r = conn.recv(BUFFER_SIZE)

+         except:

+             conn.close()

+             break

+         if not r: break

+         data += r.decode("utf-8")

+ 

+         if not sent_resp:

+             response = "Token has been handled."

+             conn.send("""HTTP/1.1 200 OK

+ Content-Length: {}

+ Content-Type: text/plain

+ Connection: Closed

+ 

+ {}""".format(len(response), response).encode("utf-8"))

+             conn.close()

+             sent_resp = True

+ 

+     s.close()

+ 

+     data = data.split("\n")

+     for line in data:

+         variables = line.split("&")

+         for var in variables:

+             kv = var.split("=")

+             if not len(kv) == 2:

+                 continue

+             if kv[0] == "access_token":

+                 return kv[1]

+     return None

@@ -87,6 +87,12 @@ 

  mock-config::

  Get the mock profile (similar to koji mock-config)

  

+ make-module::

+ Create module in Copr project (do not properly build it via MBS)

+ 

+ build-module::

+ Build module via Copr MB

+ 

  

  PROJECT ACTIONS

  ---------------
@@ -599,6 +605,40 @@ 

    copr-cli mock-config myproject fedora-rawhide-x86_64

  

  

+ MODULE ACTIONS

+ --------------

+ 

+ `copr-cli make-module [options]`

+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

+ 

+ usage: copr make-module [-h] [--yaml YAML] copr

+ 

+ Create module in Copr project (do not properly build it via MBS)

+ 

+ --yaml YAML:

+ Path to modulemd file in yaml format

+ 

+ 

+ `copr-cli build-module [options]`

+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

+ usage: copr build-module [-h] [--url URL] [--token TOKEN]

+ 

+ Build module via Copr MBS

+ 

+ --url URL:

+ SCM with modulemd file in yaml format

+ 

+ --token TOKEN:

+ OIDC token for module build service

+ 

+ 

+ EXAMPLES

+ --------

+ 

+  copr-cli make-module --yaml testmodule.yaml ownername/projectname

+  copr-cli build-module --url git://pkgs.stg.fedoraproject.org/modules/testmodule.git?#620ec77

+ 

+ 

  EXIT STATUS

  -----------

  Normally, the exit code is 0 when everything goes well. But if not, we could get:

file modified
+35 -20
@@ -10,6 +10,7 @@ 

  import sys

  import os

  import logging

+ import io

  

  import requests

  import six
@@ -21,9 +22,9 @@ 

  # urlparse from six is not available on el7

  # because it requires at least python-six-1.4.1

  if sys.version_info[0] == 2:

-     from urlparse import urlparse

+     from urlparse import urlparse, urljoin

  else:

-     from urllib.parse import urlparse

+     from urllib.parse import urlparse, urljoin

  

  if sys.version_info < (2, 7):

      class NullHandler(logging.Handler):
@@ -588,24 +589,6 @@ 

  

          return response

  

-     def create_new_build_module(self, projectname, modulemd, username=None):

-         api_endpoint = "module/build"

-         ownername = username if username else self.username

-         f = open(modulemd, "rb")

-         data = {"modulemd": (os.path.basename(f.name), f, "application/x-rpm"), "username": ownername}

- 

-         url = "{0}/coprs/{1}/{2}/{3}/".format(

-             self.api_url, ownername, projectname, api_endpoint

-         )

- 

-         def fetch(url, data, method):

-             m = MultipartEncoder(data)

-             monit = MultipartEncoderMonitor(m, lambda x: x)

-             return self._fetch(url, monit, method="post", headers={'Content-Type': monit.content_type})

- 

-         # @TODO Refactor process_package_action to be general general purpose

-         response = self.process_package_action(url, ownername, projectname, data=data, fetch_functor=fetch)

-         return response

  

      #########################################################

      ###                   Package actions                 ###
@@ -1482,3 +1465,35 @@ 

          )

          response.handle = BaseHandle(client=self, response=response)

          return response

+ 

+     def build_module(self, modulemd, token):

+         endpoint = "/module/1/module-builds/"

+         url = urljoin(self.copr_url, endpoint)

+         headers = {"Authorization": "Bearer {}".format(token)}

+ 

+         if isinstance(modulemd, io.BufferedIOBase):

+             kwargs = {"files": {"yaml": modulemd}}

+         else:

+             kwargs = {"json": {"scmurl": modulemd, "branch": "master"}}

+ 

+         response = requests.post(url, headers=headers, **kwargs)

+         return response

+ 

+     def make_module(self, projectname, modulemd, username=None):

+         api_endpoint = "module/build"

+         ownername = username if username else self.username

+         f = open(modulemd, "rb")

+         data = {"modulemd": (os.path.basename(f.name), f, "application/x-rpm"), "username": ownername}

+ 

+         url = "{0}/coprs/{1}/{2}/{3}/".format(

+             self.api_url, ownername, projectname, api_endpoint

+         )

+ 

+         def fetch(url, data, method):

+             m = MultipartEncoder(data)

+             monit = MultipartEncoderMonitor(m, lambda x: x)

+             return self._fetch(url, monit, method="post", headers={'Content-Type': monit.content_type})

+ 

+         # @TODO Refactor process_package_action to be general general purpose

+         response = self.process_package_action(url, ownername, projectname, data=data, fetch_functor=fetch)

+         return response

There are two ways how to submit a module build.

1) Submit build directly to copr-frontend
It will cheat the build. Just like our web UI. It just copies data on backend and puts yaml file in it.

copr-cli build-module testmodule.yaml --copr frostyx/foo

2) Submit build to Copr MBS
This will use MBS to orchestrate the proper module magic.

copr-cli build-module git://pkgs.stg.fedoraproject.org/modules/testmodule.git?#620ec77 --mbs

We would like to submit only local yaml file, but MBS currently does not support it. This is still decent improvement though.

copr-cli build-module testmodule.yaml --copr frostyx/foo

I think it would be useful to name the subcommand make-module, mkmodule, or create-module. Note that there is pretty much a convention in the code that ownername/projectname is a positional parameter so --copr is deviating from it.

I think it would be useful to name the subcommand make-module, mkmodule, or create-module.

Sure, I have no problem with that. Let's say make-module then ?

Note that there is pretty much a convention in the code that ownername/projectname is a positional parameter so --copr is deviating from it.

I am aware of that and I don't like it either. See, the problem is that when making module without --mbs parameter, you need to specify ownername/projectname, but when making module without it, you don't specify it, cause MBS will create new project.

In first case username/projectname would be first positional argument and modulemd second. But when using --mbs, modulemd should be first positional argument. This conflict is why it can't be positional.

In the future there won't be such --mbs parameter because all modules will be built via MBS, but I think that currently we need both methods.

What do you suggest to do, so we can drop the deviating --copr parameter? I also thought that we can have multiple commands for it, say build-module and build-module-mbs with the proper arguments that they need.

What do you suggest to do, so we can drop the deviating --copr parameter? I also thought that we can have multiple commands for it, say build-module and build-module-mbs with the proper arguments that they need.

I just meant, there would be two commands:

copr-cli make-module testmodule.yaml frostyx/foo

(or create-module or fake-build-module or ...)

and

copr-cli build-module git://pkgs.stg.fedoraproject.org/modules/testmodule.git?#620ec77

Also, actually, I think it would be good to make the first (the fake-building) command look like this:

copr-cli make-module --yaml testmodule.yaml frostyx/foo

and the second:

copr-cli build-module --something git://pkgs.stg.fedoraproject.org/module/testmodule.git?#620ec77

where 'something' is some good name for the parameter.

I agree with that. Let me update the PR

2 new commits added

  • [python][cli] rename method for making module to match cli naming
  • [cli] split module building into two separate commands
7 years ago

Updated, PR now provides following commands

copr-cli make-module --yaml testmodule.yaml ownername/projectname

and

copr-cli build-module --url git://pkgs.stg.fedoraproject.org/modules/testmodule.git?#620ec77

Great. It's looking good but could you, please, write integration tests for both methods? Something that can be launched against staging instance and confirm it's working.

@frostyx do not forget to update man page please

1 new commit added

  • [cli] describe module actions in man
7 years ago

rebased

7 years ago

4 new commits added

  • [cli][python] possibility to submit yaml file to mbs
  • [cli] compose auth url more prettier
  • [python] update auth for current MBS package
  • [frontend][python] allow to create module and it's action separately
7 years ago

rebased

7 years ago

2 new commits added

  • [cli] remove obsoleted make-module command
  • [beaker-tests-sanity] add initial tests for building modules
7 years ago

17 new commits added

  • [cli] remove obsoleted make-module command
  • [beaker-tests-sanity] add initial tests for building modules
  • [frontend] validate uploaded yaml file
  • [frontend] dont print how to use a module when it is not succeeded
  • [cli] update man for build-module command
  • [frontend] move MBS_URL to config
  • [frontend][cli][python] allow to submit optional params to mbs
  • [frontend][cli][python] frontend act as a gateway between user and mbs
  • [cli][python] possibility to submit yaml file to mbs
  • [cli] compose auth url more prettier
  • [python] update auth for current MBS package
  • [frontend][python] allow to create module and it's action separately
  • [cli] describe module actions in man
  • [python][cli] rename method for making module to match cli naming
  • [cli] split module building into two separate commands
  • [cli] add possibility to build module via MBS or not
  • [cli][python] add command for building modules
7 years ago

17 new commits added

  • [cli] remove obsoleted make-module command
  • [beaker-tests-sanity] add initial tests for building modules
  • [frontend] validate uploaded yaml file
  • [frontend] dont print how to use a module when it is not succeeded
  • [cli] update man for build-module command
  • [frontend] move MBS_URL to config
  • [frontend][cli][python] allow to submit optional params to mbs
  • [frontend][cli][python] frontend act as a gateway between user and mbs
  • [cli][python] possibility to submit yaml file to mbs
  • [cli] compose auth url more prettier
  • [python] update auth for current MBS package
  • [frontend][python] allow to create module and it's action separately
  • [cli] describe module actions in man
  • [python][cli] rename method for making module to match cli naming
  • [cli] split module building into two separate commands
  • [cli] add possibility to build module via MBS or not
  • [cli][python] add command for building modules
7 years ago

I am sorry that this PR diverged from the original goal of CLI support for submitting modules. Now it contains even changes related to using MBS and some minor modularity updates. However, I would like not to split this into multiple PRs and rather change the name of this one to be more general.

Separating would cause a lot of conflicts in the merging and also there would be a problem which commits should be moved out and which changes should be merged first, ...

Can you please review this PR again, @clime?

Err, I was just asking for tests, not for this load of commits...

I know I am sorry. But don't get scarred by the "17 new commits added". It is untrue, I've rebased and pagure just lies.

The new commits are these:

  • beed396 [cli] remove obsoleted make-module command
  • 29a3eae [beaker-tests-sanity] add initial tests for building modules
  • 1f959dc [frontend] validate uploaded yaml file
  • 21d3fc0 [frontend] dont print how to use a module when it is not succeeded
  • 89f8d36 [cli] update man for build-module command
  • 5fecccb [frontend] move MBS_URL to config
  • 5571d0c [frontend][cli][python] allow to submit optional params to mbs
  • 08301f9 [frontend][cli][python] frontend act as a gateway between user and mbs
  • 2138812 [cli][python] possibility to submit yaml file to mbs
  • fd6419b [cli] compose auth url more prettier
  • 2c9e7c4 [python] update auth for current MBS package
  • 612784d [frontend][python] allow to create module and it's action separately

where most of them are about submitting modules via CLI, but as it evolved and we are not submitting to the MBS directly, but via frontend as a gateway, things had to change in the code. It wasn't that necessary to have 1f959dc and 21d3fc0 in this branch, but other of them was actually needed.

Well, anyway, could you, please, move the unrelated commits to a separate PR(s) with proper title(s)? That way I can properly understand and review them. You can also just push them to master directly if you don't want a review.

Or are all these commits truly related to "CLI support for building modules"?

As I said in the post just above yours I think most of them is imho related. The only commit I believe it would be possible to move out of the branch and has no dependencies on any previous commit would be 21d3fc0. Let me take this one out.

                  As I said in the post just above yours I think most of them is imho related. The only commit I believe it would be possible to move out of the branch and has no dependencies on any previous commit would be 21d3fc0. Let me take this one out.

I thought you would add tests and man pages as @msuchy notes. This is pretty much what I expect to be in this PR in addition to the original changes. Could you please make it like this and move everything else to a separate PR with a proper label?

Ok, agreed. Give me a minute

10 new commits added

  • [beaker-tests-sanity] add initial tests for building modules
  • [cli] remove obsoleted make-module command
  • [cli][python] possibility to submit yaml file to mbs
  • [cli] compose auth url more prettier
  • [python] update auth for current MBS package
  • [cli] describe module actions in man
  • [python][cli] rename method for making module to match cli naming
  • [cli] split module building into two separate commands
  • [cli] add possibility to build module via MBS or not
  • [cli][python] add command for building modules
7 years ago

9 new commits added

  • [beaker-tests-sanity] add initial tests for building modules
  • [cli][python] possibility to submit yaml file to mbs
  • [cli] compose auth url more prettier
  • [python] update auth for current MBS package
  • [cli] describe module actions in man
  • [python][cli] rename method for making module to match cli naming
  • [cli] split module building into two separate commands
  • [cli] add possibility to build module via MBS or not
  • [cli][python] add command for building modules
7 years ago

It seems that we can have this set of commits and have passing test. I will move other commits to next PR.

However there is a price for this. You can't have that tests automated. It requires getting token from web browser so if you use docker container for the tests, you need to open two shells and

1) In the first one, run the Sanity/copr-cli-basic-operations/runtest-modules.sh
2) In the second, you will need to open lynx with the auth URL that copr-cli build-module command prints you

Also, it successfully submits a module to the MBS, but then I see in the logs that MBS failed to correctly build it. I am not entirely sure why, but I am not sure whether should I even solve it. Strictly speaking, module was successfully submitted and if this should be the whole point of this PR, then what happens next doesn't matter.

Well, that's a bit disappointing that the building does not actually work here. Hopefully we can fix it.

Pull-Request has been merged by clime

7 years ago