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

Stabilise openshift 4.18 on Ignition 3.4.0 spec #568

Merged
merged 3 commits into from
Dec 3, 2024

Conversation

yasminvalim
Copy link
Contributor

@yasminvalim yasminvalim commented Nov 27, 2024

Here’s what this PR aims to achieve:

  • Stabilise Openshift 4.18
  • Uses Ignition 3.4 specs
  • Add specs 4.19.0-experimental
  • Remove GRUB password support from OCP 4.18

Fixes: #564

@yasminvalim yasminvalim self-assigned this Nov 27, 2024
@yasminvalim yasminvalim force-pushed the stabilise-openshift branch 13 times, most recently from 05d4978 to f76ac57 Compare December 2, 2024 15:24
config/openshift/v4_18/schema.go Show resolved Hide resolved
config/openshift/v4_18/translate_test.go Show resolved Hide resolved
config/openshift/v4_19_exp/translate.go Outdated Show resolved Hide resolved
config/config.go Show resolved Hide resolved
internal/doc/main.go Outdated Show resolved Hide resolved
@travier
Copy link
Member

travier commented Dec 2, 2024

Commit split that should help review:

  • 1rst commit: Fix the 4.17.0 imports that we missed from a previous PR
  • 2nd commit: All the changes needed to stabilize 4.18.0, dropping down to ignition 3.4.0, fcos 1.5.0, base 0.5
  • 3rd commit: Add 4.19.0-exp, which is copied from 4.18 before the stabilization, and based on latest exp for ignition, fcos & base
  • 4th commit: Rebuild the docs

@yasminvalim yasminvalim force-pushed the stabilise-openshift branch 2 times, most recently from 924cada to f8b4b3b Compare December 2, 2024 17:39
@yasminvalim
Copy link
Contributor Author

yasminvalim commented Dec 2, 2024

Commit split that should help review:

  • 1rst commit: Fix the 4.17.0 imports that we missed from a previous PR
  • 2nd commit: All the changes needed to stabilize 4.18.0, dropping down to ignition 3.4.0, fcos 1.5.0, base 0.5
  • 3rd commit: Add 4.19.0-exp, which is copied from 4.18 before the stabilization, and based on latest exp for ignition, fcos & base
  • 4th commit: Rebuild the docs

Thanks for the guidance! I’ve reorganized the commits based on the example you provided, and it should be easier to review now. Let me know if you think any further changes are needed.

Also, do we need to remove the GRUB password support like we did in the last stabilization? If so, I’ll create a separate commit for that change.

@travier
Copy link
Member

travier commented Dec 2, 2024

Yes, 4.18 will not have GRUB password support in the MCO so we need to remove it there.

@yasminvalim
Copy link
Contributor Author

Yes, 4.18 will not have GRUB password support in the MCO so we need to remove it there.

Nice, I added one more commit to remove it. I think we are good now, the commit are properly organized.

@travier
Copy link
Member

travier commented Dec 3, 2024

Failing on:

Error: config/openshift/v4_18/translate.go:307:6: func `translateUserGrubCfg` is unused (unused)
  func translateUserGrubCfg(config *types.Config, ts *translate.TranslationSet) translate.TranslationSet {
       ^

Copy link
Member

@travier travier left a comment

Choose a reason for hiding this comment

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

Some more splitting to make this easier to review. I'll do the update.

config/openshift/v4_18/translate.go Outdated Show resolved Hide resolved
config/config.go Show resolved Hide resolved
internal/docs/config-openshift-v4_10.md Outdated Show resolved Hide resolved
docs/config-openshift-v4_19-exp.md Show resolved Hide resolved
internal/doc/main.go Outdated Show resolved Hide resolved
@travier travier force-pushed the stabilise-openshift branch from 82ed846 to 93e44b2 Compare December 3, 2024 14:57
- Dropping down to ignition 3.4.0, fcos 1.5.0, base 0.5
- Remove GRUB password support

Fixes: coreos#564
@travier travier force-pushed the stabilise-openshift branch from 93e44b2 to e78a040 Compare December 3, 2024 15:02
Copy link
Member

@travier travier left a comment

Choose a reason for hiding this comment

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

This LGTM now. @prestist Can I let you take a final quick look and merge this? Thanks

@yasminvalim
Copy link
Contributor Author

@travier Thank you!

@prestist
Copy link
Collaborator

prestist commented Dec 3, 2024

@travier sure thing :) thank you both for your efforts on this, looking now

Copy link
Collaborator

@prestist prestist left a comment

Choose a reason for hiding this comment

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

LGTM, I dont see any issues. Great work all, Thank you for getting this done 🚀

@yasminvalim yasminvalim merged commit f1eabb5 into coreos:main Dec 3, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Stabilise openshift 4.18 on ignition 3.4.0 spec
3 participants