fix for flux plugin: clean up old charts from chart cache after repo update #4115 #5644
Conversation
✅ Deploy Preview for kubeapps-dev canceled.Built without sensitive environment variables
|
|
could one of you guys review this when you get a chance? Thanks |
| return c.deleteChartsHelper(repo, sets.String{}) | ||
| } | ||
|
|
||
| func (c *ChartCache) PurgeObsoleteChartVersions(keepThese []models.Chart) error { |
There was a problem hiding this comment.
Just reading the code below, I think keepThese is the slice of charts for which you're wanting to keep all versions? (as in, below, each chart is iterated with the redis key for each version added to the set of keys to keep for that repo, which is then passed as keep to deleteChartsHelper).
I must be missing something here, because it looks like it only ever calls deleteChartsHelper with repos of charts that were in the keepThese slice? I expected to find a function that purges all other repositories, other than the ones you wanted to keep (maybe it does and I'm just mis-reading).
EDIT: Oh - reading further down, I think I understand: this function is called when re-importing charts after an update to the repo, for example, so keepThese is actually populated from the new data, meaning that if the new data no longer includes a certain version, it'll get purged here. OK, I think I was confused initially because it wasn't clear to me that keepThese was the new data that hadn't been synced yet. (?)
There was a problem hiding this comment.
yes, you got it
| repos := map[types.NamespacedName]sets.String{} | ||
| for _, ch := range keepThese { | ||
| if ch.Repo == nil { | ||
| continue |
There was a problem hiding this comment.
Should that be an error if this function is called with a chart with a nil repo? Or at least, logging the issue? Wondering why we'd not want to report that somehow?
There was a problem hiding this comment.
shouldn't ever happen, but you're right. Let me at least log it
|
thank you for reviewing |
fix for long-standing issue #4115
plus DRYing up a bunch of existing test code
Not 100% sure who to ask for review. Its not urgent. If no-one materializes in the next few days, I will ask
The only thing of interest here is really a new func call
executed before an old call to
There is a new integration test I added for this scenario.
Most everything else is test code cleanup and doesn't need any review