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

Fix RFE-4185: run bundle-upgrade with long image names #6477

Merged

Conversation

nunnatsa
Copy link
Contributor

@nunnatsa nunnatsa commented Jun 25, 2023

Fix #6476

Description of the change

In run bundle-upgrade, hash the cache directory name to avoid error of too long file name.

Motivation for the change

run bundle-upgrade is failing if the bundle image names are long. See #6476

Checklist

If the pull request includes user-facing changes, extra documentation is required:

@nunnatsa nunnatsa temporarily deployed to deploy June 25, 2023 14:58 — with GitHub Actions Inactive
@nunnatsa nunnatsa temporarily deployed to deploy June 25, 2023 14:58 — with GitHub Actions Inactive
@nunnatsa nunnatsa temporarily deployed to deploy June 25, 2023 14:58 — with GitHub Actions Inactive
@nunnatsa nunnatsa temporarily deployed to deploy June 25, 2023 14:58 — with GitHub Actions Inactive
@nunnatsa nunnatsa temporarily deployed to deploy June 25, 2023 14:58 — with GitHub Actions Inactive
@nunnatsa nunnatsa temporarily deployed to deploy June 25, 2023 14:58 — with GitHub Actions Inactive
@nunnatsa nunnatsa temporarily deployed to deploy June 25, 2023 14:58 — with GitHub Actions Inactive
@nunnatsa nunnatsa temporarily deployed to deploy June 25, 2023 17:28 — with GitHub Actions Inactive
@nunnatsa nunnatsa temporarily deployed to deploy June 25, 2023 17:28 — with GitHub Actions Inactive
@nunnatsa nunnatsa temporarily deployed to deploy June 25, 2023 17:28 — with GitHub Actions Inactive
@nunnatsa nunnatsa temporarily deployed to deploy June 25, 2023 17:28 — with GitHub Actions Inactive
@nunnatsa nunnatsa temporarily deployed to deploy June 25, 2023 17:28 — with GitHub Actions Inactive
@nunnatsa nunnatsa temporarily deployed to deploy June 25, 2023 17:28 — with GitHub Actions Inactive
@nunnatsa nunnatsa temporarily deployed to deploy June 25, 2023 17:28 — with GitHub Actions Inactive
@nunnatsa nunnatsa temporarily deployed to deploy June 26, 2023 04:57 — with GitHub Actions Inactive
@nunnatsa nunnatsa temporarily deployed to deploy June 26, 2023 04:57 — with GitHub Actions Inactive
@nunnatsa nunnatsa temporarily deployed to deploy June 26, 2023 04:57 — with GitHub Actions Inactive
@nunnatsa nunnatsa temporarily deployed to deploy June 26, 2023 04:57 — with GitHub Actions Inactive
@nunnatsa nunnatsa temporarily deployed to deploy June 26, 2023 04:57 — with GitHub Actions Inactive
@nunnatsa nunnatsa temporarily deployed to deploy June 26, 2023 04:57 — with GitHub Actions Inactive
@nunnatsa nunnatsa temporarily deployed to deploy June 26, 2023 04:57 — with GitHub Actions Inactive
Copy link
Contributor

@everettraven everettraven left a comment

Choose a reason for hiding this comment

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

Overall looks good to me. Just a couple nits (non-blocking IMO but would be nice to have addressed)

internal/olm/fbcutil/util_test.go Outdated Show resolved Hide resolved
Comment on lines +178 to +179
hashBytes := hash.Sum(nil)
return fmt.Sprintf("%x", hashBytes)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we shorten this to something like:

Suggested change
hashBytes := hash.Sum(nil)
return fmt.Sprintf("%x", hashBytes)
return string(hash.Sum(nil))

?

If not this is fine, but I think those are equivalent and doesn't require use of the fmt.Sprintf function

Copy link
Contributor Author

@nunnatsa nunnatsa Jun 26, 2023

Choose a reason for hiding this comment

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

Thanks for the comment @everettraven - but these two forms are not equivalent. string(hash.Sum(nil)) will try to use the random bytes as characters. The result is unexpected - we'll end up with wrong UTF-8 chars, non-printable chars and so on.

The original format generates a hex string (2 hex digits per byte), which is a safe string.

@nunnatsa nunnatsa temporarily deployed to deploy June 26, 2023 13:01 — with GitHub Actions Inactive
@nunnatsa nunnatsa temporarily deployed to deploy June 26, 2023 13:01 — with GitHub Actions Inactive
@nunnatsa nunnatsa temporarily deployed to deploy June 26, 2023 13:01 — with GitHub Actions Inactive
@nunnatsa nunnatsa temporarily deployed to deploy June 26, 2023 13:01 — with GitHub Actions Inactive
@nunnatsa nunnatsa temporarily deployed to deploy June 26, 2023 13:01 — with GitHub Actions Inactive
@nunnatsa nunnatsa temporarily deployed to deploy June 26, 2023 13:01 — with GitHub Actions Inactive
@nunnatsa nunnatsa temporarily deployed to deploy June 26, 2023 13:01 — with GitHub Actions Inactive
Copy link
Member

@rashmigottipati rashmigottipati left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jun 26, 2023
@rashmigottipati rashmigottipati merged commit a99d7ed into operator-framework:master Jun 26, 2023
28 checks passed
@nunnatsa nunnatsa deleted the address-RFE-4185 branch June 27, 2023 05:18
sebsoto added a commit to sebsoto/release that referenced this pull request Sep 20, 2023
This copies the workflow that is used for the 4.13->4.14 upgrade, using
Azure instead of vSphere. Azure is being used as we wish to test storage
migration which occurs only for Azure across this upgrade.

The operator-sdk version was bumped to include this required fix:
operator-framework/operator-sdk#6477

`make update` was ran to generate the presubmit file
openshift-merge-robot pushed a commit to openshift/release that referenced this pull request Sep 20, 2023
This copies the workflow that is used for the 4.13->4.14 upgrade, using
Azure instead of vSphere. Azure is being used as we wish to test storage
migration which occurs only for Azure across this upgrade.

The operator-sdk version was bumped to include this required fix:
operator-framework/operator-sdk#6477

`make update` was ran to generate the presubmit file
jrvaldes added a commit to jrvaldes/release that referenced this pull request Jan 25, 2024
This commit updates the operator-sdk version to include
a required fix. operator-framework/operator-sdk#6477
jrvaldes added a commit to jrvaldes/release that referenced this pull request Jan 28, 2024
This commit updates the operator-sdk version to include
a required fix. operator-framework/operator-sdk#6477
jrvaldes added a commit to jrvaldes/release that referenced this pull request Jan 29, 2024
This commit updates the operator-sdk version to include
a required fix. operator-framework/operator-sdk#6477
jrvaldes added a commit to jrvaldes/release that referenced this pull request Jan 30, 2024
This commit updates the operator-sdk version to include
a required fix. operator-framework/operator-sdk#6477
jrvaldes added a commit to jrvaldes/release that referenced this pull request Feb 5, 2024
This commit updates the operator-sdk version to include
a required fix. operator-framework/operator-sdk#6477
jrvaldes added a commit to jrvaldes/release that referenced this pull request Feb 14, 2024
This commit updates the operator-sdk version to include
a required fix. operator-framework/operator-sdk#6477
jrvaldes added a commit to jrvaldes/release that referenced this pull request Feb 16, 2024
This commit updates the operator-sdk version to include
a required fix. operator-framework/operator-sdk#6477
openshift-merge-bot bot pushed a commit to openshift/release that referenced this pull request Feb 16, 2024
* wmco: bump operator-sdk in release 4.12

This commit updates the operator-sdk version to include
a required fix. operator-framework/operator-sdk#6477

* wmco: add azure upgrade test to release 4.12

This commit introduces the azure-e2e-upgrade test
in the release 4.12 branch by upgrading from the
last released 4.12 bundle to the pipeline bundle.
mshitrit pushed a commit to mshitrit/release that referenced this pull request Feb 18, 2024
* wmco: bump operator-sdk in release 4.12

This commit updates the operator-sdk version to include
a required fix. operator-framework/operator-sdk#6477

* wmco: add azure upgrade test to release 4.12

This commit introduces the azure-e2e-upgrade test
in the release 4.12 branch by upgrading from the
last released 4.12 bundle to the pipeline bundle.
shaior pushed a commit to natifridman/release that referenced this pull request Feb 20, 2024
* wmco: bump operator-sdk in release 4.12

This commit updates the operator-sdk version to include
a required fix. operator-framework/operator-sdk#6477

* wmco: add azure upgrade test to release 4.12

This commit introduces the azure-e2e-upgrade test
in the release 4.12 branch by upgrading from the
last released 4.12 bundle to the pipeline bundle.
sgoveas pushed a commit to sgoveas/release that referenced this pull request Feb 22, 2024
* wmco: bump operator-sdk in release 4.12

This commit updates the operator-sdk version to include
a required fix. operator-framework/operator-sdk#6477

* wmco: add azure upgrade test to release 4.12

This commit introduces the azure-e2e-upgrade test
in the release 4.12 branch by upgrading from the
last released 4.12 bundle to the pipeline bundle.
memodi pushed a commit to memodi/release that referenced this pull request Mar 14, 2024
* wmco: bump operator-sdk in release 4.12

This commit updates the operator-sdk version to include
a required fix. operator-framework/operator-sdk#6477

* wmco: add azure upgrade test to release 4.12

This commit introduces the azure-e2e-upgrade test
in the release 4.12 branch by upgrading from the
last released 4.12 bundle to the pipeline bundle.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fail to run bundle-upgrade if the bundle image names are too long
3 participants