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

This fixes an issue where catalog/charts were stale after a repo was updated #7672

Merged
merged 1 commit into from
Dec 9, 2022

Conversation

codyrancher
Copy link
Contributor

@codyrancher codyrancher commented Dec 9, 2022

Summary

The catalog.cattle.io.clusterrepos/{repo} resource doesn't subscribe to updates. We now force a full request in the areas where it's appropriate to do so.

fixes #7668

Testing

If you want me to modify the repo for you just let me know. If you want to test it yourself the following notes should be helpful.

To test this you'll need a chart repo that you can modify. I created a simple repo at https://github.com/CodyrancherTest/charts/tree/master.
To modify do the following:

  • fork that repo
  • modify the values.yaml (I just change the logLevel)
  • run helm package . within the root of the directory
  • Add, commit and push all of the changes to your github repo

…updated

The catalog.cattle.io.clusterrepos/{repo} resource doesn't subscribe to updates. We now force a full request in the areas where it's appropriate to do so.

fixes rancher#7668
@codyrancher codyrancher merged commit 5d75ccf into rancher:master Dec 9, 2022
@AndrewHoffmanQA
Copy link

@codyrancher I will gladly take you up on the offer to modify the repro, would you be interested in walking through it on a call?

I have a couple questions about the order of operations for this bug and repro steps on this work item vs the actual ticket and would benefit from the KT in general.

@catherineluse
Copy link
Contributor

catherineluse commented Jan 19, 2023

@codyrancher I'm working on a related issue and have a question. Since we are now force fetching all chart repos on most chart pages, does that make the "Refresh" button redundant? (The button in the linked issue on the git repositories list page) I'm thinking either the button should be removed, or we should do less force-fetching elsewhere to justify the existence of the button and improve performance for setups with a huge number of helm chart apps.

@codyrancher
Copy link
Contributor Author

@codyrancher I'm working on a related issue and have a question. Since we are now force fetching all chart repos on most chart pages, does that make the "Refresh" button redundant? (The button in the linked issue on the git repositories list page) I'm thinking either the button should be removed, or we should do less force-fetching elsewhere to justify the existence of the button and improve performance for setups with a huge number of helm chart apps.

@catherineluse I don't think the refresh button is redundant. I'm having to force refresh the data because the resource doesn't receive automatic updates over the socket. If you want to do less force fetching for performance reasons I think that's a reasonable goal but I think it will technically be wrong since the data could be stale.

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.

Chart Repository refresh does not actually refresh the chart repo
4 participants