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.

Sign up for GitHub

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

Jump to bottom

feat: change to use helm get values -a to get image #7479

Merged
merged 3 commits into from
Feb 11, 2025
Merged

feat: change to use helm get values -a to get image #7479

merged 3 commits into from
Feb 11, 2025

Conversation

FrankYang0529
Copy link
Member

@FrankYang0529 FrankYang0529 commented Feb 3, 2025 •
edited
Loading

IMPORTANT: Please do not create a Pull Request without creating an issue first.

Problem:
After bumping Rancher to v2.10.1, we can't get helm values from App objects.

Solution:
Change to use helm get values -a directly, so we don't need to rely on Rancher code.

Related Issue:
#7320

Test plan:

  1. Build an ISO with feat: bump RKE2 to v1.31.4+rke2r1 and Rancher to v2.10.1 harvester-installer#926.
  2. Create a 3-node airgapped cluster without error.
  3. Check all nodes are in control plane.
  4. Create support bundle without error.

@FrankYang0529 FrankYang0529 mentioned this pull request Feb 3, 2025
Merged
@FrankYang0529 FrankYang0529 requested a review from bk201 February 3, 2025 08:26
@FrankYang0529 FrankYang0529 force-pushed the HARV-7320 branch 6 times, most recently from c5613c3 to f25b0e4 Compare February 7, 2025 03:29
@innobead
Copy link
Contributor

innobead commented Feb 10, 2025 •
edited
Loading

After bumping Rancher to v2.10.1, we can't get helm values from App objects.

I am curious about the reason behind this breaking change at Rancher. Do we have any information about it?

innobead
innobead approved these changes Feb 10, 2025
View reviewed changes
Copy link
Contributor

@innobead innobead left a comment

Choose a reason for hiding this comment

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

LGTM

@FrankYang0529
Copy link
Member Author

I am curious about the reason behind this breaking change at Rancher. Do we have any information about it?

This is observed result. I can't find source code about it.

WebberHuang1118
WebberHuang1118 approved these changes Feb 11, 2025
View reviewed changes
Copy link
Member

@WebberHuang1118 WebberHuang1118 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks the PR.

@FrankYang0529 FrankYang0529 merged commit f802414 into harvester:master Feb 11, 2025
9 of 10 checks passed
@FrankYang0529 FrankYang0529 deleted the HARV-7320 branch February 11, 2025 10:17
@FrankYang0529 FrankYang0529 added backport-to/v1.4 For PR only, automatically create a backport PR when master PR is merged. backport-to/v1.5 For PR only, automatically create a backport PR when master PR is merged. labels Feb 11, 2025
This was referenced Feb 11, 2025
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@innobead innobead innobead approved these changes

@bk201 bk201 bk201 approved these changes

@WebberHuang1118 WebberHuang1118 WebberHuang1118 approved these changes

@ibrokethecloud ibrokethecloud ibrokethecloud approved these changes

Assignees
No one assigned
Labels
backport-to/v1.4 For PR only, automatically create a backport PR when master PR is merged. backport-to/v1.5 For PR only, automatically create a backport PR when master PR is merged.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@FrankYang0529 @innobead @bk201 @WebberHuang1118 @ibrokethecloud