-
Notifications
You must be signed in to change notification settings - Fork 81
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
CLID-232,OCPBUGS-38339,OCPBUGS-38343,OCPBUGS-38233: Create release signature configmap #924
Conversation
@lmzuccarelli: This pull request references CLID-232 which is a valid jira issue. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
/retest |
0391217
to
2401c6d
Compare
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.
Hi @lmzuccarelli,
Thanks so much for the PR, I left a few comments, especially for go.work.sum.
Let' s discuss them together.
@lmzuccarelli: This pull request references CLID-232 which is a valid jira issue. This pull request references Jira Issue OCPBUGS-38339, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
Requesting review from QA contact: The bug has been updated to refer to the pull request using the external bug tracker. This pull request references Jira Issue OCPBUGS-38343, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
Requesting review from QA contact: The bug has been updated to refer to the pull request using the external bug tracker. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
9c335c4
to
818ebf7
Compare
818ebf7
to
ae1d6a7
Compare
@lmzuccarelli: This pull request references CLID-232 which is a valid jira issue. This pull request references Jira Issue OCPBUGS-38339, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: This pull request references Jira Issue OCPBUGS-38343, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
@lmzuccarelli: This pull request references CLID-232 which is a valid jira issue. This pull request references Jira Issue OCPBUGS-38339, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: This pull request references Jira Issue OCPBUGS-38343, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
ae1d6a7
to
7d8abc5
Compare
@lmzuccarelli: This pull request references CLID-232 which is a valid jira issue. This pull request references Jira Issue OCPBUGS-38339, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: The bug has been updated to refer to the pull request using the external bug tracker. This pull request references Jira Issue OCPBUGS-38343, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: The bug has been updated to refer to the pull request using the external bug tracker. This pull request references Jira Issue OCPBUGS-38233, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
Requesting review from QA contact: The bug has been updated to refer to the pull request using the external bug tracker. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
@lmzuccarelli: This pull request references CLID-232 which is a valid jira issue. This pull request references Jira Issue OCPBUGS-38339, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: This pull request references Jira Issue OCPBUGS-38343, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: This pull request references Jira Issue OCPBUGS-38233, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
v2/internal/pkg/release/signature.go
Outdated
|
||
cr := clusterresources.New(o.Log, o.Opts.Global.WorkingDir, o.Config, "") | ||
err = cr.GenerateSignatureConfigMap(digest, id, data) | ||
if err != nil { | ||
o.Log.Error("%v", err) | ||
} | ||
|
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.
Currently all the clusterresources are being generated in the executor.go, the generation of the SignaturesConfigMap could follow the same standard.
Here is an example:
oc-mirror/v2/internal/pkg/cli/executor.go
Line 781 in 8d037ba
if !o.Opts.IsDryRun { |
Having the generation of the SignatureConfigMap in the executor could bring some benefits:
- The SignatureConfigMap will be generated only if the workflow finished successfully (in the collector it will be always generated)
- If the customer is running a dry run, having it in the executor.go will avoid the generation of the SignatureConfigMap (in the collector it will be always generated)
- Having the clusterresources being generated at only one place is easy to maintain the code in the long term
- Since the executor already has the dependency on the clusterresources pkg it is possible to avoid an additional dependency of the clusterresources pkg in the collector pkg
- Having the release collector calling the clusterresources (write results) pkg directly will break the architecture defined here: https://github.com/openshift/oc-mirror/blob/main/v2/assets/architecture.png where the executor should be the responsible to call the clusterresources (write results)
02fd1d3
to
783ff4b
Compare
} | ||
// read the signatures directory | ||
sigDir := filepath.Join(o.WorkingDir, signatureDir) | ||
signatures, err := os.ReadDir(sigDir) |
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.
Hi @lmzuccarelli
Wouldn't we be creating configMaps for all the releases that were ever mirrored in this manner?
Another possibility here would be to use something similar to CatalogSourceGenerator: we could use the release image digests to track the signature files needed?
WDYT?
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.
Yes - actually its simpler than that I''ll just add to the hashmap created for the BinaryData field . Good catch
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.
Hi @lmzuccarelli
I like the separation of concerns much better this way!
Thanks
783ff4b
to
5849c94
Compare
/hold |
/unhold |
@lmzuccarelli: This pull request references CLID-232 which is a valid jira issue. This pull request references Jira Issue OCPBUGS-38339, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: This pull request references Jira Issue OCPBUGS-38343, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: This pull request references Jira Issue OCPBUGS-38233, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
@lmzuccarelli: This pull request references CLID-232 which is a valid jira issue. This pull request references Jira Issue OCPBUGS-38339, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: This pull request references Jira Issue OCPBUGS-38343, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: This pull request references Jira Issue OCPBUGS-38233, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
@lmzuccarelli: This pull request references CLID-232 which is a valid jira issue. This pull request references Jira Issue OCPBUGS-38339, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: This pull request references Jira Issue OCPBUGS-38343, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: This pull request references Jira Issue OCPBUGS-38233, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
@lmzuccarelli: This pull request references CLID-232 which is a valid jira issue. This pull request references Jira Issue OCPBUGS-38339, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: This pull request references Jira Issue OCPBUGS-38343, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: This pull request references Jira Issue OCPBUGS-38233, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
@lmzuccarelli: This pull request references CLID-232 which is a valid jira issue. This pull request references Jira Issue OCPBUGS-38339, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: This pull request references Jira Issue OCPBUGS-38343, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: This pull request references Jira Issue OCPBUGS-38233, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
Completed testing CLID-232 and found two issues, listing the same here:
|
@kasturinarra - thanks for the feedback , I'll update and fix asap. |
bc04692
to
2076959
Compare
@kasturinarra - I have updated as requested and addressed the version-arch prefix problem |
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.
Thanks @lmzuccarelli
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: lmzuccarelli, sherine-k The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
2076959
to
b251068
Compare
/lgtm |
@lmzuccarelli: all tests passed! Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
@lmzuccarelli: Jira Issue OCPBUGS-38339: All pull requests linked via external trackers have merged: Jira Issue OCPBUGS-38339 has been moved to the MODIFIED state. Jira Issue OCPBUGS-38343: All pull requests linked via external trackers have merged: Jira Issue OCPBUGS-38343 has been moved to the MODIFIED state. Jira Issue OCPBUGS-38233: All pull requests linked via external trackers have merged: Jira Issue OCPBUGS-38233 has been moved to the MODIFIED state. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
[ART PR BUILD NOTIFIER] Distgit: oc-mirror-plugin |
…gnature configmap (openshift#924) * Update to address issues from review * Add yaml support * Update to move logic to executor * Update to tie release tag and digest for signature configmap * Update form QE feedback
@lmzuccarelli: Jira Issue OCPBUGS-38339 is in an unrecognized state (ON_QA) and will not be moved to the MODIFIED state. Jira Issue OCPBUGS-38343: All pull requests linked via external trackers have merged: Jira Issue OCPBUGS-38343 has been moved to the MODIFIED state. Jira Issue OCPBUGS-38233 is in an unrecognized state (ON_QA) and will not be moved to the MODIFIED state. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
Description
Add the release signature configmap for oc-mirror v2 (json format compatible with v1) to the cluster-resources folder of oc-mirror
This fix also addresses the bug fixes for OCPBUGS-38339 and OCPBUGS-38343
Type of change
How Has This Been Tested?
Use the following imagesetconfig
Execute oc-mirror mirror to disk workflow
Now execute the disk to mirror workflow
Expected Outcome
Check the working-dir/cluster-resources folder there should be a files with naming convention
signature-configmap.json and signature-configmap.yaml
To verify the contents of this field execute the following bash cli The
working-dir/signatures/
folder has the same digestThe value output should match the contents of the config map (BinaryData section)
Verify the bug fixes
OCPBUGS-38339
In the jira ticket follow the steps to re-produce the bug, there should be no panic but an error with appropriate message
OCPBUGS-38343
Once the disk-to-mirror workflow has been executed , verify that here should be both json and yaml files (with configmap structure) in the directory "working-dir/cluster-resources"
OCPBUGS-38233
Once the disk-to-mirror workflow has been executed , verify that here should be both json and yaml files (with configmap structure) in the directory "working-dir/cluster-resources"