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

Add size/md5 to Azure metadata #2038

Closed
wants to merge 1 commit into from

Conversation

cgwalters
Copy link
Member

First this changes ore to write a JSON file that the Python side
parses, so we don't need to re-synthesize the URL on the Python
side.

Gather Azure Blob metadata (size+md5) and add that to our metadata,
This is mainly useful so that one can use the Azure API to "offline validate"
against our metadata snapshot. I'd also like to add our sha256
checksum for consistency, but that can come later.

This is prep for coreos/stream-metadata-go#13

First this changes ore to write a JSON file that the Python side
parses, so we don't need to re-synthesize the URL on the Python
side.

Gather Azure Blob metadata (size+md5) and add that to our metadata,
This is mainly useful so that one can use the Azure API to "offline validate"
against our metadata snapshot.  I'd also like to add our sha256
checksum for consistency, but that can come later.

This is prep for coreos/stream-metadata-go#13
@openshift-ci-robot
Copy link

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cgwalters

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@cgwalters
Copy link
Member Author

/test all
Draft since I haven't actually tested this and CI doesn't cover this really.

@cgwalters
Copy link
Member Author

I think the direction of this PR makes sense, but I'd like to not block on it because it makes everything a lot harder to roll out if we have to respin RHCOS too and can't use the existing metadata. (Well, I guess I could retro-patch the existing metadata, but eh)

The value is also fairly low because I think few people are actually going to validate the image offline.

@bgilbert
Copy link
Contributor

AIUI, users (or tools) won't be interacting with the VHD bits directly, just using them as a copy source within Azure and then creating a VM image from the copy. In that case, since the URL is essentially an identifier, I don't think it's that important to include digests etc.. So yeah, the value seems low to me too.

However, I think we should consider uploaded artifacts immutable, if only to reduce confusion. If we need to respin a release, we should just update stream metadata to point to the new image. That's what it's for. 🙂

@openshift-ci-robot
Copy link

@cgwalters: PR needs rebase.

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/test-infra repository.

@openshift-ci
Copy link

openshift-ci bot commented Nov 25, 2021

@cgwalters: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/sanity fb25382 link /test sanity
ci/prow/images fb25382 link true /test images
ci/prow/rhcos fb25382 link true /test rhcos

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/test-infra repository. I understand the commands that are listed here.

@dustymabe
Copy link
Member

Closing as this is Draft for a long time. Re-open if it gets refreshed.

@dustymabe dustymabe closed this Dec 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants