Skip to content
This repository has been archived by the owner on Oct 13, 2023. It is now read-only.

ART-3050: full assembly awareness for plashet from-tags #439

Merged
merged 3 commits into from
Jun 28, 2021

Conversation

vfreex
Copy link
Contributor

@vfreex vfreex commented Jun 18, 2021

When an artist is trying to refine an assembly with a basis event, existing assembly-specific builds for that assembly are not canonically part of the assembly until the artist pins then in the assembly metadata with "is". It follows that plashet should honor "is" when set for RPMs in the assembly metadata when constructing a repo.

Another change in behavior is that plashet should honor group "dependencies" defined in the assembly configuration. 431 takes care of determining what a group configuration should look like in light of a given assembly name on the command line. Unlike normal group.yml, assembly group definitions in releases.yml can add "dependencies" to the loaded models.

NOTE:

  1. Plashet are able to create both rhel8 and rhel7 repos, but rpm nvrs defined in assembly config don't specify which repo they should go. I think we can assume assembly-specific nvrs either pinned by "is" or given by group dependencies are always for RHEL8. Let me know if this is wrong. Update: Now we have per rhel version dependencies in releases.yml.
  2. It seems to me that we shouldn't allow "is" in a wildcard rpm member entry. Let me know if this is wrong. Update: This is now handled by assembly_metadata_config()
  3. If the nvr pinned by "is" is embargoed but we are building an non-embargoed plashet, an error will be raised.
  4. Remove --brew-event command line argument from from-tags subverb since we need to honor the event id defined in the basis.
  5. This PR requires Assembly lense #431.
  6. I used the following releases.yml for testing:
releases:
  art1:
    assembly:
      basis: 
        brew_event: 39473862
      group:
        dependencies:
          rpms:
          - el8: cri-o-1.21.0-88.rhaos4.8.gitfd485de.el8
      members:
        rpms:
        - distgit_key: openshift-clients
          metadata:
            is:
              el8: openshift-clients-4.9.0-202106031118.p0.git.714295d.assembly.stream.el8
        - distgit_key: openshift-kuryr
          metadata:
            is:
              el8: openshift-kuryr-4.9.0-202105211327.p0.git.be00acd.el8

@openshift-ci openshift-ci bot requested a review from Ximinhan June 18, 2021 12:07
@vfreex vfreex force-pushed the assemblies-plashet-full-awareness branch 2 times, most recently from 23f35a0 to 95ddf47 Compare June 18, 2021 15:32
@openshift-eng openshift-eng deleted a comment from openshift-bot Jun 18, 2021
@openshift-eng openshift-eng deleted a comment from openshift-bot Jun 18, 2021
@vfreex vfreex force-pushed the assemblies-plashet-full-awareness branch from 95ddf47 to 47ee1f1 Compare June 18, 2021 15:43
@openshift-eng openshift-eng deleted a comment from openshift-bot Jun 18, 2021
@jupierce
Copy link
Contributor

Plashet are able to create both rhel8 and rhel7 repos, but rpm nvrs defined in assembly config don't specify which repo they should go. I think we can assume assembly-specific nvrs either pinned by "is" or given by group dependencies are always for RHEL8. Let me know if this is wrong.

Nice catch. I think this is a failing of the schema. I've changed the hackmd to use:

dependencies:
  rpms:
  - el7: <nvr>
  - el8: <nr>
  - el9: <entries like this are not too far in the future>

This should allow you to find only NVRs relevant to the repo's RHEL version. I've also changed the schema for is when pinning member components:

is:
  el7:  <nvr>
  el8:  <nvr>

get_latest_build in 431 has been updated to honor this. plashet could pass el_target=# in order for the right rpm build record to be returned.

For pinning image components, is remains a nvr:

is:
  nvr: <nvr>

Let me know what you think.

It seems to me that we shouldn't allow "is" in a wildcard rpm member entry. Let me know if this is wrong.

I agree, but I would consider it deep human error and not something I would worry too much about defending against.

If the nvr pinned by "is" is embargoed but we are building an non-embargoed plashet, an error will be raised.

Perfect.

Remove --brew-event command line argument from from-tags subverb since we need to honor the event id defined in the basis.

Great. Perfect place to rely on Runtime.

@openshift-ci openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 22, 2021
When an artist is trying to refine an assembly with a basis event, existing assembly-specific builds for that assembly are not canonically part of the assembly until the artist pins then in the assembly metadata with "is". It follows that plashet should honor "is" when set for RPMs in the assembly metadata when constructing a repo.

Another change in behavior is that plashet should honor group "dependencies" defined in the assembly configuration. 431 takes care of determining what a group configuration should look like in light of a given assembly name on the command line. Unlike normal group.yml, assembly group definitions in releases.yml can add "dependencies" to the loaded models.
@vfreex vfreex force-pushed the assemblies-plashet-full-awareness branch from 47ee1f1 to 5b55f91 Compare June 23, 2021 08:46
@openshift-ci openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 23, 2021
doozerlib/cli/plashet.py Outdated Show resolved Hide resolved
doozerlib/cli/plashet.py Outdated Show resolved Hide resolved
doozerlib/cli/plashet.py Outdated Show resolved Hide resolved
doozerlib/cli/plashet.py Outdated Show resolved Hide resolved
doozerlib/cli/plashet.py Outdated Show resolved Hide resolved
doozerlib/cli/plashet.py Outdated Show resolved Hide resolved
doozerlib/cli/plashet.py Outdated Show resolved Hide resolved
@vfreex vfreex force-pushed the assemblies-plashet-full-awareness branch from 568bf33 to 4024b40 Compare June 24, 2021 02:57
@vfreex
Copy link
Contributor Author

vfreex commented Jun 24, 2021

@jupierce I've addressed all issues you mentioned. Please have a look again.

Co-authored-by: Justin Pierce <jupierce@redhat.com>
@vfreex vfreex force-pushed the assemblies-plashet-full-awareness branch from 4024b40 to 0dd7a31 Compare June 24, 2021 09:00
@vfreex vfreex force-pushed the assemblies-plashet-full-awareness branch 2 times, most recently from e501239 to dc68686 Compare June 25, 2021 09:25
@vfreex
Copy link
Contributor Author

vfreex commented Jun 25, 2021

@jupierce I pushed another commit to extract some logic in from-tags subverb into a separate module so it can be reused in for-assembly verb. Could you have a look again?

@vfreex vfreex force-pushed the assemblies-plashet-full-awareness branch from dc68686 to ba974b0 Compare June 25, 2021 10:05
@openshift-eng openshift-eng deleted a comment from openshift-bot Jun 25, 2021
@openshift-eng openshift-eng deleted a comment from openshift-bot Jun 25, 2021
@openshift-eng openshift-eng deleted a comment from openshift-bot Jun 25, 2021
@openshift-eng openshift-eng deleted a comment from openshift-bot Jun 25, 2021
@openshift-eng openshift-eng deleted a comment from openshift-bot Jun 25, 2021
@openshift-eng openshift-eng deleted a comment from openshift-bot Jun 25, 2021
@openshift-eng openshift-eng deleted a comment from openshift-bot Jun 25, 2021
So that future development can reuse it.

Also limit the NVRs for signing to builds from tags and those pinned by
"is", because we are not able to sign non-OCP RPMs, which should already
be signed.
@openshift-bot
Copy link

Build #11

GLOB sdist-make: /mnt/workspace/jenkins/working/art-tools_doozer_PR-439/setup.py
py3 inst-nodeps: /mnt/workspace/jenkins/working/art-tools_doozer_PR-439/.tox/.tmp/package/1/rh-doozer-1.3.6.zip
py3 installed: aiofiles==0.7.0,appdirs==1.4.4,astroid==2.5.6,attrs==21.2.0,autopep8==1.5.7,bashlex==0.15,cached-property==1.5.2,certifi==2021.5.30,cffi==1.14.5,chardet==4.0.0,click==8.0.1,contextvars==2.4,coverage==5.5,cryptography==3.4.7,decorator==5.0.9,Deprecated==1.2.12,distlib==0.3.2,dockerfile-parse==1.2.0,filelock==3.0.12,flake8==3.9.2,flexmock==0.10.4,future==0.18.2,gssapi==1.6.13,idna==2.10,immutables==0.15,importlib-metadata==4.5.0,importlib-resources==5.1.4,iniconfig==1.1.1,isort==5.8.0,kobo==0.19.0,koji==1.25.0,lazy-object-proxy==1.6.0,mccabe==0.6.1,mock==4.0.3,mysql-connector-python==8.0.25,packaging==20.9,pluggy==0.13.1,protobuf==3.17.3,py==1.10.0,pycodestyle==2.7.0,pycparser==2.20,pyflakes==2.3.1,pygit2==1.6.0,PyGithub==1.55,PyJWT==2.1.0,pykerberos==1.2.1,pylint==2.8.3,PyNaCl==1.4.0,pyparsing==2.4.7,pytest==6.2.4,python-dateutil==2.8.1,PyYAML==5.4.1,requests==2.25.1,requests-gssapi==1.2.3,requests-kerberos==0.12.0,rh-doozer @ file:///mnt/workspace/jenkins/working/art-tools_doozer_PR-439/.tox/.tmp/package/1/rh-doozer-1.3.6.zip,rpm==4.14.3,rpm-py-installer==1.1.0,semver==2.13.0,six==1.16.0,tenacity==7.0.0,toml==0.10.2,tox==3.23.1,typed-ast==1.4.3,typing==3.7.4.3,typing-extensions==3.10.0.0,urllib3==1.26.5,virtualenv==20.4.7,wrapt==1.12.1,zipp==3.4.1
py3 run-test-pre: PYTHONHASHSEED='1507879825'
py3 run-test: commands[0] | flake8
py3 run-test: commands[1] | coverage run --branch --source doozerlib -m unittest discover -t . -s tests/
..............................Environment variables required for db operation missing. Doozer will be runningin no DB use mode.
.Environment variables required for db operation missing. Doozer will be runningin no DB use mode.
.Environment variables required for db operation missing. Doozer will be runningin no DB use mode.
.Environment variables required for db operation missing. Doozer will be runningin no DB use mode.
.........................................s.s....................................s.s...s.s.s..s.s.s...........................................................................................ERROR:doozer.tests.test_runtime:Unable to check if target branch branch exists: whatever
.................
----------------------------------------------------------------------
Ran 229 tests in 1.517s

OK (skipped=10)
py3 run-test: commands[2] | coverage report
Name                                   Stmts   Miss Branch BrPart  Cover
------------------------------------------------------------------------
doozerlib/__init__.py                     11      4      2      1    62%
doozerlib/assembly.py                     76      7     48      6    90%
doozerlib/assertion.py                    22      0      6      0   100%
doozerlib/brew.py                        329    193    146      3    39%
doozerlib/build_status_detector.py        76      4     46      2    93%
doozerlib/cli/__init__.py                100     45     20      0    46%
doozerlib/cli/__main__.py               1156   1156    478      0     0%
doozerlib/cli/cli_opts.py                 16      3      6      0    86%
doozerlib/cli/config_plashet.py          442    442    190      0     0%
doozerlib/cli/detect_embargo.py          163     35     70      8    75%
doozerlib/cli/images_health.py            79     28     24      2    61%
doozerlib/cli/images_streams.py          556    556    232      0     0%
doozerlib/cli/release_gen_payload.py     304    212    141      2    26%
doozerlib/cli/rpms_build.py              160     55     54      7    58%
doozerlib/cli/scan_sources.py            152    122     80      2    16%
doozerlib/config.py                       98     98     44      0     0%
doozerlib/constants.py                     9      0      0      0   100%
doozerlib/coverity.py                    237    209     74      0     9%
doozerlib/dblib.py                       248    152     68      4    35%
doozerlib/distgit.py                    1340    886    616     13    31%
doozerlib/dotconfig.py                    56     44     32      0    14%
doozerlib/exceptions.py                    3      0      0      0   100%
doozerlib/exectools.py                   194     75     76     17    56%
doozerlib/gitdata.py                     168    133     76      0    14%
doozerlib/image.py                       257    214    128      2    12%
doozerlib/logutil.py                      10      1      2      1    83%
doozerlib/metadata.py                    412    166    186     26    54%
doozerlib/model.py                       111     21     36      2    82%
doozerlib/olm/__init__.py                  0      0      0      0   100%
doozerlib/olm/bundle.py                  217    217     36      0     0%
doozerlib/operator_metadata.py           364     65     66      6    79%
doozerlib/plashet.py                      80      5     46      8    90%
doozerlib/pushd.py                        21      2      2      0    91%
doozerlib/repos.py                       207    105    113     17    44%
doozerlib/rhcos.py                        36      9      6      0    74%
doozerlib/rpm_builder.py                 214     30    111     30    80%
doozerlib/rpmcfg.py                      148     59     60      8    55%
doozerlib/runtime.py                     828    601    336      6    22%
doozerlib/source_modifications.py         89     28     18      3    65%
doozerlib/state.py                        24     12      8      0    38%
doozerlib/util.py                        289    135    102      4    50%
------------------------------------------------------------------------
TOTAL                                   9302   6129   3785    180    31%
___________________________________ summary ____________________________________
  py3: commands succeeded
  congratulations :)

@vfreex
Copy link
Contributor Author

vfreex commented Jun 28, 2021

Merging so that future development can base on this.

@vfreex vfreex merged commit ef4392e into openshift-eng:master Jun 28, 2021
vfreex added a commit that referenced this pull request Jul 6, 2021
58c406f [ART-3093] Allow assemblies to specify machine-os-content in gen-payload (#460)
beea167 Fix error message (#461)
6b7c51c Replace mojo links with The Source (#459)
5d71b80 Update doozerlib/plashet.py
d8f0956 Update doozerlib/plashet.py
2568f67 Address unittest failures
4c72f79 [ART-2687] Expose brew event and brew tag to modification scripts
03fb7a7 Allow gen-payload to honor metadata.is
d75c8e5 package_name instead of component_name
70271e6 Fix golang image build
e9e24fc Allow --repo to take precedence over --repo-type defaults (#452)
fd94360 Add additional event-safe methods for koji
0b3ccb0 ART-3050: add plashet from-tags --rhcos
395d222 [ART-3075] Add plashet subverb "for-assembly" (#449)
ef4392e ART-3050: full assembly awareness for plashet from-tags (#439)
537043b Test dependency accumulation
0617168 Ensure inherited member list entries can contribute metadata
2efccc2 Open PRs for base images
417bd11 Fix method docs
6aadeb2 ART-3034: Implement doozer images:rebase --force-yum-updates (#428)
2009bd8 Testing fixes
89d2cad fix multiple-default click option error
345f848 Fix version extraction
1834dc3 A bit more bundle doc
f89b434 Randomize assignee to avoid bugging approver[0]
bc08bbb Lock parent images to assembly basis
e6ff58f Code to merge image/rpm metadata with assembly overrides
vfreex added a commit that referenced this pull request Jul 6, 2021
58c406f [ART-3093] Allow assemblies to specify machine-os-content in gen-payload (#460)
beea167 Fix error message (#461)
6b7c51c Replace mojo links with The Source (#459)
5d71b80 Update doozerlib/plashet.py
d8f0956 Update doozerlib/plashet.py
2568f67 Address unittest failures
4c72f79 [ART-2687] Expose brew event and brew tag to modification scripts
03fb7a7 Allow gen-payload to honor metadata.is
d75c8e5 package_name instead of component_name
70271e6 Fix golang image build
e9e24fc Allow --repo to take precedence over --repo-type defaults (#452)
fd94360 Add additional event-safe methods for koji
0b3ccb0 ART-3050: add plashet from-tags --rhcos
395d222 [ART-3075] Add plashet subverb "for-assembly" (#449)
ef4392e ART-3050: full assembly awareness for plashet from-tags (#439)
537043b Test dependency accumulation
0617168 Ensure inherited member list entries can contribute metadata
2efccc2 Open PRs for base images
417bd11 Fix method docs
6aadeb2 ART-3034: Implement doozer images:rebase --force-yum-updates (#428)
2009bd8 Testing fixes
89d2cad fix multiple-default click option error
345f848 Fix version extraction
1834dc3 A bit more bundle doc
f89b434 Randomize assignee to avoid bugging approver[0]
bc08bbb Lock parent images to assembly basis
e6ff58f Code to merge image/rpm metadata with assembly overrides
vfreex added a commit that referenced this pull request Jul 6, 2021
58c406f [ART-3093] Allow assemblies to specify machine-os-content in gen-payload (#460)
beea167 Fix error message (#461)
6b7c51c Replace mojo links with The Source (#459)
5d71b80 Update doozerlib/plashet.py
d8f0956 Update doozerlib/plashet.py
2568f67 Address unittest failures
4c72f79 [ART-2687] Expose brew event and brew tag to modification scripts
03fb7a7 Allow gen-payload to honor metadata.is
d75c8e5 package_name instead of component_name
70271e6 Fix golang image build
e9e24fc Allow --repo to take precedence over --repo-type defaults (#452)
fd94360 Add additional event-safe methods for koji
0b3ccb0 ART-3050: add plashet from-tags --rhcos
395d222 [ART-3075] Add plashet subverb "for-assembly" (#449)
ef4392e ART-3050: full assembly awareness for plashet from-tags (#439)
537043b Test dependency accumulation
0617168 Ensure inherited member list entries can contribute metadata
2efccc2 Open PRs for base images
417bd11 Fix method docs
6aadeb2 ART-3034: Implement doozer images:rebase --force-yum-updates (#428)
2009bd8 Testing fixes
89d2cad fix multiple-default click option error
345f848 Fix version extraction
1834dc3 A bit more bundle doc
f89b434 Randomize assignee to avoid bugging approver[0]
bc08bbb Lock parent images to assembly basis
e6ff58f Code to merge image/rpm metadata with assembly overrides
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants