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

Process Tekton results instead of labels and annotations #231

Merged
merged 2 commits into from
Feb 24, 2022

Conversation

chmeliik
Copy link
Contributor

@chmeliik chmeliik commented Feb 22, 2022

More details in individual commits

Maintainers will complete the following section

  • Commit messages are descriptive enough
  • Code coverage from testing does not decrease and new code is covered
  • [n/a] JSON/YAML configuration changes are updated in the relevant schema
  • Pull request has a link to an osbs-docs PR for user documentation updates

CLOUDBLD-8889

OSBS pipelines will provide the most important build results as Tekton
results. Fetch repositories and koji_build_id from Tekton results rather
than labels and annotations.

Previously, koji_build_id was handled as a string (it was taken directly
from OpenShift labels without converting to integer). In the results
returned by the get_build_results method, koji-build-id is an integer.
Convert it to string explicitly to preserve backward compatibility.

Signed-off-by: Adam Cmiel <acmiel@redhat.com>
@chmeliik chmeliik marked this pull request as ready for review February 22, 2022 16:30
@chmeliik
Copy link
Contributor Author

PTAL, depends on containerbuildsystem/osbs-client#1076 but otherwise should be done-ish

This will need a docs update, will work on that soon

We will also want to follow up on removing the task_annotations_whitelist from atomic-reactor config, I'll either do it now or file a separate story for it

task_annotations = {
# TODO: previously, remote_sources was double JSON-encoded, do we need to keep that?
# prev: {"remote_sources": "[{\\"name\\": null, \\"url\\": \\"some url\\"}]"}
# curr: {"remote_sources": [{"name": null, "url": "some url"}]}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How much do we need to worry about keeping build_annotations.json exactly the same as before? I'm thinking not that much, since when we last switched from remote_source_url to remote_sources, nobody seemed to notice

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the other hand, it's easier to preserve the format than to answer this question. I'll change this to match the previous format

CLOUDBLD-8889

Due to unstable behavior of updating annotations on Tekton PipelineRun
objects, using build annotations to generate build_annotations.json is
not reliable. Instead, generate build_annotations.json from Tekton
results.

Drop the concept of an annotations whitelist, simply hardcode the list
of results to be uploaded.

For backward compatibility, keep the build_annotations.json name even
though the file now contains Tekton results rather than annotations. The
file used to contain raw values of annotations - usually JSON-encoded
strings. Encode result values explicitly to preserve the same format.

Signed-off-by: Adam Cmiel <acmiel@redhat.com>
@chmeliik
Copy link
Contributor Author

docs pr: containerbuildsystem/osbs-docs#207

@chmeliik
Copy link
Contributor Author

Packit is stuck on Fedora 36, merging anyway

@chmeliik chmeliik merged commit 0667b30 into containerbuildsystem:master Feb 24, 2022
@chmeliik chmeliik deleted the tekton-results branch February 24, 2022 09:34
@chmeliik
Copy link
Contributor Author

Release notes:

  • Task results (repositories, koji_build_id) are now taken from Tekton results rather than labels and annotations
  • The build_annotations.json file in task output is generated from Tekton results rather than annotations
    • It is no longer possible to configure the included annotations via the koji_task_annotations_whitelist

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants