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

Kubeapps Helm AppRepository should support configurable sync interval #3661

Closed
absoludity opened this issue Oct 26, 2021 · 2 comments · Fixed by #5599
Closed

Kubeapps Helm AppRepository should support configurable sync interval #3661

absoludity opened this issue Oct 26, 2021 · 2 comments · Fixed by #5599
Assignees
Labels
component/apprepository Issue related to kubeapps apprepository kind/feature An issue that reports a feature (approved) to be implemented

Comments

@absoludity
Copy link
Contributor

Description:

Currently the interval with which the helm repositories referenced by an AppRepository are synced to the cache is configurable only for the apprepository controller itself (Values.apprepository.crontab), defaulting to 10minute intervals.

This should be exposed as an option on the AppRepository itself so that the crontab can be specified per repository, given that different repositories have different needs.

Additional information you deem important (e.g. issue happens only occasionally):

This has come up because the new API will support an interval when creating an app repository (as supported by flux (and carvel?)). The Helm plugin/support can initially just not expose this in the UI and fail validation if specified to the API, so it's not urgent, but would be nice to have.

@absoludity absoludity added component/apprepository Issue related to kubeapps apprepository kind/feature An issue that reports a feature (approved) to be implemented priority/low labels Oct 26, 2021
@antgamdia antgamdia added the next-iteration Issues to be discussed in planning session label Jul 16, 2022
@antgamdia
Copy link
Contributor

As of today, the new Repos API implementation for Helm does not support it:

// Helm repositories do not support intervals for reconciliation by now
if repo.interval != "" {
return nil, status.Errorf(codes.InvalidArgument, "interval is not supported for Helm repositories")
}

@ppbaena
Copy link
Collaborator

ppbaena commented Oct 17, 2022

@antgamdia take a look at #5128 as it is related.

antgamdia added a commit that referenced this issue Nov 3, 2022
### Description of the change

While working on #3661, I noticed some files were plenty of typos 😅 , so
I squeezed some fixes in my PR. However, I gave up and ended up running
an auto-fixer. This PR is adding all the occurrences of the auto-fix
tool.

### Benefits

No more typos for a while, hopefully.

### Possible drawbacks

This is a one-time change, but we should prevent them from happening.

### Applicable issues

N/A

### Additional information

We could have a look at the corresponding GHA that already exists:
https://github.com/marketplace/actions/typos-action

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>
absoludity added a commit that referenced this issue Nov 17, 2022
### Description of the change

This PR is adding a new field (`Interval`) in the `AppRepository` CRD.
This field will override the global default `crontab` used for every
repository, which is aligned with the recent Repositories API we
defined.

The PR includes every required change across the components, namely:

- Add `Interval` field in the CR definition.
- Add this field when spinning up jobs in the `apprepository-controller`
and use it; otherwise, use the default one.
- Use this value in the AddRepo/UpdateRepo methods of the Repos API
(Helm plugin)
- Remove any workarounds preventing the UI from setting the interval in
Helm repos.

### Benefits

Helm repos can have customizable intervals now.

### Possible drawbacks

~IRL test is still pending. I don't know if adding a field in the CRD is
breaking something, though we already added some fields in the past with
no impact, but want to re-check just in case.~

### Applicable issues

- fixes #3661 

### Additional information

After the IRL test, some notes:

- Helm does not upgrade the CRDs, so you have to delete and install
them.
- If the CRD is outdated, then the interval is not being saved, falling
back to the previous, classical behavior.
- One caveat is that the upgrade process: since we are creating a
CronJob and passing the crontab value on creation via the apprepository
controller, from the Repos API we don't have a clear way to _edit_ the
cronjob once generated.
- In this PR we've addressed it by just disabling the field when
upgrading

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>
Co-authored-by: Michael Nelson <minelson@vmware.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/apprepository Issue related to kubeapps apprepository kind/feature An issue that reports a feature (approved) to be implemented
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants