-
Notifications
You must be signed in to change notification settings - Fork 61
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
feat(RELEASE-1243): make create advisory task idempotent #817
base: development
Are you sure you want to change the base?
Conversation
Skipping CI for Draft Pull Request. |
ad3378b
to
46960e2
Compare
/retest |
2 similar comments
/retest |
/retest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks close, but I think it is missing something. Take this scenario for example:
already published advisory:
content:
images:
- containerImage: foo@sha256:sha2
tags:
- 1.0.0
new advisory_json contains
content:
images:
- containerImage: foo@sha256:sha2
tags:
- 1.0.0
- containerImage: bar@sha256:sha
tags:
- 1.0.0
In this case,
- containerImage: bar@sha256:sha
tags:
- 1.0.0
should be published in a new advisory. That is to say, the entry that already exists with containerImage and tags matching is dropped, but there is still new stuff so an advisory has to be filed, just with less
SHIP_DATE=$(date -u +"%Y-%m-%dT%H:%M:%SZ") | ||
YEAR=${SHIP_DATE%%-*} # derive the year from the ship date | ||
# Define advisory directory | ||
ADVISORY_BASE_DIR="advisories/data/advisories/$(params.origin)/${YEAR}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm I think even if it was released last year, we shouldn't again, so maybe drop $YEAR here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, on a side note
I am just wondering if we want to add any limit on existing advisory checks? Like the last 10 or 20? because I'm just thinking if some product has 100s of advisories, it will go through each to find if it is duplicate or not, and this will repeat for each containerImage.
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a fair point. I think because you narrow it down to the params.origin, it won't be too bad, but it could still happen eventually. Maybe don't add a limit here beyond origin, but then create a followup jira to add a limit and we can discuss it as a team in refinement?
46960e2
to
5ca601e
Compare
Signed-off-by: Happy Bhati <[email protected]>
5ca601e
to
b4ee0be
Compare
Thanks @johnbieren for explaining the use case. I am working on fixing and testing it, and I will ping you once it is ready for review. |
@happybhati: The following test has Failed, say /retest to rerun failed tests.
Inspecting Test ArtifactsTo inspect your test artifacts, follow these steps:
mkdir -p oras-artifacts
cd oras-artifacts
oras pull quay.io/konflux-test-storage/konflux-team/release-service-catalog:konflux-e2e-tests-catalog-54j8h Test results analysis🚨 Failed to provision a cluster, see the log for more details: Click to view logsINFO: Log in to your Red Hat account... INFO: Configure AWS Credentials... WARN: The current version (1.2.47) is not up to date with latest rosa cli released version (1.2.50). WARN: It is recommended that you update to the latest version. INFO: Logged in as 'konflux-ci-418295695583' on 'https://api.openshift.com' INFO: Create ROSA with HCP cluster... WARN: The current version (1.2.47) is not up to date with latest rosa cli released version (1.2.50). WARN: It is recommended that you update to the latest version. INFO: Creating cluster 'kx-0abd115d2b' INFO: To view a list of clusters and their status, run 'rosa list clusters' INFO: Cluster 'kx-0abd115d2b' has been created. INFO: Once the cluster is installed you will need to add an Identity Provider before you can login into the cluster. See 'rosa create idp --help' for more information.
Nodes:
INFO: Preparing to create operator roles. |
Describe your changes
Relevant Jira
Checklist before requesting a review
do not merge
label if there's a dependency PRrelease-service-maintainers
handle if you are unsure who to tagSigned-off-by: My name <email>