Skip to content

Update Karmada dependencies to v1.14.1 #235

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

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

devarsh10
Copy link
Contributor

What type of PR is this?

What this PR does / why we need it:

  • This PR makes the dashboard to use the upgraded version of karmada which is 1.14.1 from 1.13.0

Which issue(s) this PR fixes:
Fixes #221

Special notes for your reviewer:

  • I have tried on my machine to deploy dashboard and it worked like a charm. Until,
  • I made the changes recloned the modified dashboard and I was not able to see the version in dashboard UI.
  • Earlier I was seeing it, when I was using the dashboard main branch.

Does this PR introduce a user-facing change?:
NONE

Thank you

@karmada-bot karmada-bot added the do-not-merge/contains-merge-commits Indicates a PR which contains merge commits. label Jul 24, 2025
@karmada-bot
Copy link
Collaborator

Welcome @devarsh10! It looks like this is your first PR to karmada-io/dashboard 🎉

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Summary of Changes

Hello @devarsh10, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request performs a routine dependency upgrade, specifically updating the Karmada dependency to its latest stable version. This ensures the dashboard leverages the most recent features and fixes provided by the Karmada project, maintaining compatibility and benefiting from upstream improvements.

Highlights

  • Dependency Upgrade: Upgraded the github.com/karmada-io/karmada dependency from version v1.13.0 to v1.14.1.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in issue comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@karmada-bot karmada-bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jul 24, 2025
Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request aims to upgrade the karmada dependency from v1.13.0 to v1.14.1. While the change in go.mod is straightforward, it appears to be incomplete as it introduces a regression where the version is no longer displayed in the dashboard UI, as noted by the author. My review highlights that dependency upgrades often require corresponding changes in other parts of the codebase, and I've suggested looking into the backend API, Helm chart configuration, and ensuring the go.sum file is correctly updated to resolve this issue.

@@ -8,7 +8,7 @@ require (
github.com/glebarez/sqlite v1.11.0
github.com/gobuffalo/flect v1.0.2
github.com/golang-jwt/jwt/v5 v5.2.1
github.com/karmada-io/karmada v1.13.0
github.com/karmada-io/karmada v1.14.1

Choose a reason for hiding this comment

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

high

Hi, thanks for this contribution.

While this change correctly updates the karmada dependency version, it appears to be incomplete and has introduced a regression, as you've pointed out in the PR description (the version is no longer visible in the UI).

Dependency upgrades, especially across minor versions (e.g., 1.13.x to 1.14.x), can include API changes that require corresponding updates in the application code.

To resolve the regression, please investigate the following:

  • Backend Code: The API for fetching the version might have changed in Karmada v1.14.1. The backend code that exposes this information to the UI likely needs to be updated to align with the new Karmada API.
  • Helm Chart: The appVersion in charts/karmada-dashboard/Chart.yaml may need to be updated. It's currently 1.16.0, which seems inconsistent with the Karmada version 1.14.1. Please verify if this needs to be changed to 1.14.1 or another appropriate version.
  • go.sum file: The go.sum file must be updated to reflect the dependency changes. Please run go mod tidy and commit the resulting changes to go.sum.

Addressing these points should help fix the issue you've observed.

@devarsh10 devarsh10 force-pushed the dependency-upgrade branch from 3a36bb0 to b1433f4 Compare July 24, 2025 13:53
devarsh10 and others added 5 commits July 24, 2025 14:02
Co-authored-by: MdSayemkhan <[email protected]>
Signed-off-by: warjiang <[email protected]>
Signed-off-by: Devarsh <[email protected]>
Co-authored-by: MdSayemkhan <[email protected]>
Signed-off-by: warjiang <[email protected]>
Signed-off-by: Devarsh <[email protected]>
Co-authored-by: MdSayemkhan <[email protected]>
Signed-off-by: warjiang <[email protected]>
Signed-off-by: Devarsh <[email protected]>
@devarsh10 devarsh10 force-pushed the dependency-upgrade branch from b1433f4 to 1f39d19 Compare July 24, 2025 14:03
@karmada-bot karmada-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. do-not-merge/contains-merge-commits Indicates a PR which contains merge commits. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed do-not-merge/contains-merge-commits Indicates a PR which contains merge commits. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 24, 2025
Signed-off-by: Devarsh Shah<[email protected]>
Signed-off-by: Devarsh <[email protected]>
@devarsh10 devarsh10 force-pushed the dependency-upgrade branch from a64eab9 to 8cbc46c Compare July 28, 2025 06:28
@karmada-bot karmada-bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed do-not-merge/contains-merge-commits Indicates a PR which contains merge commits. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jul 28, 2025
@karmada-bot
Copy link
Collaborator

Adding label do-not-merge/contains-merge-commits because PR contains merge commits, which are not allowed in this repository.
Use git rebase to reapply your commits on top of the target branch. Detailed instructions for doing so can be found here.

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-sigs/prow repository.

@karmada-bot karmada-bot added do-not-merge/contains-merge-commits Indicates a PR which contains merge commits. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jul 28, 2025
Signed-off-by: Devarsh Shah <[email protected]>
Signed-off-by: Devarsh <[email protected]>
@karmada-bot karmada-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jul 28, 2025
Signed-off-by: Devarsh Shah <[email protected]>
Signed-off-by: Devarsh <[email protected]>
@warjiang
Copy link
Contributor

@devarsh10 could you squash the commits, I think the PR need at most two commits, but it seems the PR contains commits that not belongs you.

@@ -265,7 +265,7 @@ func accessClusterInPullMode(opts *pullModeOption) error {
// TODO: deployment ready cannot exactly express that cluster is ready
// It should also check cluster resource on karmada control-plane
// maybe karmadactl should optimized it
if err = cmdutil.WaitForDeploymentRollout(opts.memberClusterClient, karmadaAgentDeployment, int(timeout)); err != nil {
if err = cmdutil.WaitForDeploymentRollout(opts.memberClusterClient, karmadaAgentDeployment, time.Duration(timeout)); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

it seems that there's no need to add type cast, the type of timeout is alreay time.Duration

Signed-off-by: Devarsh <[email protected]>
@devarsh10 devarsh10 force-pushed the dependency-upgrade branch from 8641b9e to 56f077f Compare July 31, 2025 04:20
@warjiang
Copy link
Contributor

warjiang commented Aug 2, 2025

@devarsh10 I've tested it locally, and overview screenshot as following:

image

I thinks is PR is ready to be merged, but you should clean the commits:
image

@warjiang
Copy link
Contributor

warjiang commented Aug 2, 2025

/assign

@@ -289,32 +289,38 @@ func accessClusterInPushMode(opts *pushModeOption) error {

controlPlaneKubeClient := kubeclient.NewForConfigOrDie(opts.karmadaRestConfig)
memberClusterKubeClient := kubeclient.NewForConfigOrDie(opts.memberClusterRestConfig)
id, err := karmadautil.ObtainClusterID(memberClusterKubeClient)

// Fix 1: Use separate variable assignment, then assign to struct field
Copy link
Member

Choose a reason for hiding this comment

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

What does the Fix 1 mean here?
Also the Fix 2 in below?

@RainbowMango
Copy link
Member

/retitle Update Karmada dependencies to v1.14.1

@karmada-bot karmada-bot changed the title chore: dependency upgrade Update Karmada dependencies to v1.14.1 Aug 4, 2025
@karmada-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from warjiang. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found 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

klog.ErrorS(err, "Check ClusterIdentify failed")
registerOption.ClusterID = clusterID

// BETTER FIX 2: Use the existing client from opts
Copy link
Member

Choose a reason for hiding this comment

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

Again, what's the BETTER FIX 2 mean?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge/contains-merge-commits Indicates a PR which contains merge commits. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade dependency karmada v1.14
4 participants