#3230 hub: whitelist arch-agnostic logs for image tasks
Merged 2 years ago by tkopecek. Opened 2 years ago by tkopecek.
tkopecek/koji issue3229  into  master

file modified
+5 -5
@@ -10385,14 +10385,14 @@ 

      logs = [f for f in os.listdir(workpath) if f.endswith('.log')]

      for logfile in logs:

          logsrc = joinpath(workpath, logfile)

-         if tinfo['method'] == 'livemedia':

-             # multiarch livemedia spins can have log name conflicts, so we

-             # add the arch to the path

+         if tinfo['method'] in ('appliance', 'image', 'indirectionimage', 'livecd'):

              logdir = joinpath(koji.pathinfo.build(build_info),

-                               'data/logs/image', imgdata['arch'])

+                               'data/logs/image')

          else:

+             # multiarch livemedia (and plugins') spins can have log name conflicts, so we

+             # add the arch to the path

              logdir = joinpath(koji.pathinfo.build(build_info),

-                               'data/logs/image')

+                               'data/logs/image', imgdata['arch'])

          koji.ensuredir(logdir)

          final_path = joinpath(logdir, os.path.basename(logfile))

          if os.path.exists(final_path):

@@ -101,7 +101,7 @@ 

          for arch in data:

              taskdir = koji.pathinfo.task(data[arch]['task_id'])

              os.makedirs(taskdir)

-             filenames = data[arch]['files'] +  data[arch]['logs']

+             filenames = data[arch]['files'] + data[arch]['logs']

              for filename in filenames:

                  path = os.path.join(taskdir, filename)

                  with open(path, 'wt', encoding='utf-8') as fp:
@@ -117,7 +117,7 @@ 

              for filename in data[arch]['files']:

                  paths.append(os.path.join(imgdir, filename))

              for filename in data[arch]['logs']:

-                 paths.append(os.path.join(logdir, 'image', filename))

+                 paths.append(os.path.join(logdir, 'image', arch, filename))

          # bdir = koji.pathinfo.build(buildinfo)

          # paths.append(os.path.join(bdir, 'metadata.json'))

          return paths

@@ -109,7 +109,7 @@ 

          # Check that the log symlink made it to where it was supposed to.

          dest = os.readlink(workdir + '/foo.log')

          dest = os.path.abspath(os.path.join(workdir, dest))

-         self.assertEqual(dest, self.tempdir + '/data/logs/image/foo.log')

+         self.assertEqual(dest, self.tempdir + '/data/logs/image/x86_64/foo.log')

  

          # And.. check all the sql statements

          self.assertEqual(len(cursor.execute.mock_calls), 1)

@mikem I would like to completely drop first part of condition and create arch dir for all tasks (even for image). Do you think it is ok?

I would like to completely drop first part of condition and create arch dir for all tasks (even for image). Do you think it is ok?

I was briefly concerned that there could be a case of external code relying on importImage and not providing the arch field, but it appears that this is explicitly required in the docstring, and relied upon in the underlying import_image() function. So I think it's ok to assume that it's there.

It looks like get_build_logs will handle this fine. It doesn't care what the log layout it; it just walks the logdir and reports the logs it finds.

From a purely Koji pespective, I think this is ok.

My only remaining concern is that CI systems could conceivably be looking at particular paths for logs. This is technically a backwards incompatible change, though certainly arguably better.

Is there a reason for dropping the first condition besides making the code a little smaller? Unless there is, I guess I'd err on the side of just merging this as-is.

No, it is just consistence/cleanup. So, let's gowith merge as is.

rebased onto a02550c

2 years ago

Commit bc04708 fixes this pull-request

Pull-Request has been merged by tkopecek

2 years ago

Metadata Update from @jcupova:
- Pull-request tagged with: testing-ready

2 years ago

Metadata Update from @jobrauer:
- Pull-request tagged with: testing-done

2 years ago