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

Replace image validity check with shell script #3898

Closed
wants to merge 1 commit into from

Conversation

twz123
Copy link
Member

@twz123 twz123 commented Jan 8, 2024

Description

The script is more or less straight forward and saves us some go dependencies and a custom go build execution.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update

How Has This Been Tested?

  • Manual test
  • Auto test added

Checklist:

  • My code follows the style guidelines of this project
  • My commit messages are signed-off
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules
  • I have checked my code and corrected any misspellings

The script is more or less straight forward and saves us some go
dependencies and a custom go build execution.

Signed-off-by: Tom Wieczorek <[email protected]>
@twz123 twz123 force-pushed the validate-images-shell-script branch from cd761de to e3d81a0 Compare January 9, 2024 08:13
@twz123
Copy link
Member Author

twz123 commented Jan 9, 2024

On the other hand, is this functionality still useful? K0s ships its own images anyways, so it's under the project's control to provide appropriate multi-arch images anyways. We could simply remove it?

@jnummelin
Copy link
Member

We could simply remove it?

I wonder would we catch any missing archs in smokes? For most of the images probably yes, but we're not really running with all permutation configs for arm/arm64 smokes that would exercise all the images.

@twz123
Copy link
Member Author

twz123 commented Jan 9, 2024

My point here is that the default images are all built by the k0s project. How likely is it that we're forgetting to ship an architecture? I guess this was a bigger concern back in the day when k0s used upstream images verbatim.

@twz123
Copy link
Member Author

twz123 commented Jan 9, 2024

Plus, wouldn't the airgap image tarball generation fail when an image is unavailable for a certain architecture?

@jnummelin
Copy link
Member

How likely is it that we're forgetting to ship an architecture?

Quite low I'd say

wouldn't the airgap image tarball generation fail when an image is unavailable for a certain architecture?

I think you're right, it would AFAIK fail.

So maybe we should just remove this stuff

@twz123
Copy link
Member Author

twz123 commented Jan 10, 2024

Superseded by #3913.

@twz123 twz123 closed this Jan 10, 2024
auto-merge was automatically disabled January 10, 2024 11:44

Pull request was closed

@twz123 twz123 deleted the validate-images-shell-script branch January 10, 2024 11:44
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.

2 participants