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

refactor: create root level HelmConfig struct which applies to all HelmReleases #356

Merged
merged 4 commits into from
Jul 31, 2024

Conversation

ahmad-ibra
Copy link
Contributor

@ahmad-ibra ahmad-ibra commented Jul 30, 2024

Issue

First step into resolving

Description

In an effort to simplify the TUI prompts in validatorctl to not require lots of re-prompts for the helm configuration, we first need to refactor our helm related structs.

The proposed structure would be this:

  • ValidatorConfigSpec.HelmConfig defines the helm registry configuration that would be used to pull all plugin charts
  • ValidatorConfigSpec.Plugins has been simplifies to only specify the charts repository, name, version, and values.

Along side this change, I updated the reviewable-ext make target to also update the CRDs in the validator helm chart

Copy link

codecov bot commented Jul 30, 2024

Codecov Report

Attention: Patch coverage is 31.57895% with 13 lines in your changes missing coverage. Please review.

Files Patch % Lines
api/v1alpha1/zz_generated.deepcopy.go 0.00% 9 Missing ⚠️
internal/controller/validatorconfig_controller.go 71.42% 1 Missing and 1 partial ⚠️
pkg/helm/helm.go 0.00% 1 Missing and 1 partial ⚠️
@@            Coverage Diff             @@
##             main     #356      +/-   ##
==========================================
- Coverage   48.90%   48.56%   -0.34%     
==========================================
  Files          22       22              
  Lines        1274     1287      +13     
==========================================
+ Hits          623      625       +2     
- Misses        584      594      +10     
- Partials       67       68       +1     
Files Coverage Δ
api/v1alpha1/validatorconfig_types.go 100.00% <ø> (ø)
pkg/helm/options.go 60.00% <100.00%> (ø)
internal/controller/validatorconfig_controller.go 68.55% <71.42%> (+0.27%) ⬆️
pkg/helm/helm.go 40.81% <0.00%> (-0.57%) ⬇️
api/v1alpha1/zz_generated.deepcopy.go 8.10% <0.00%> (-0.42%) ⬇️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4c85249...5153f7c. Read the comment docs.

@ahmad-ibra ahmad-ibra marked this pull request as ready for review July 30, 2024 22:27
@ahmad-ibra ahmad-ibra requested a review from a team as a code owner July 30, 2024 22:27
@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Jul 30, 2024
@dosubot dosubot bot added the enhancement Enhancement to an existing feature label Jul 30, 2024
TylerGillson
TylerGillson previously approved these changes Jul 30, 2024
api/v1alpha1/validatorconfig_types.go Outdated Show resolved Hide resolved
chart/validator/templates/validator-config.yaml Outdated Show resolved Hide resolved
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Jul 30, 2024
@ahmad-ibra ahmad-ibra marked this pull request as ready for review July 31, 2024 01:13
@ahmad-ibra ahmad-ibra merged commit 8eadfe1 into main Jul 31, 2024
7 of 8 checks passed
@ahmad-ibra ahmad-ibra deleted the refactor/root-helm-config branch July 31, 2024 07:03
ahmad-ibra pushed a commit that referenced this pull request Jul 31, 2024
🤖 I have created a release *beep* *boop*
---


##
[0.0.51](v0.0.50...v0.0.51)
(2024-07-31)


### Refactoring

* create root level HelmConfig struct which applies to all HelmReleases
([#356](#356))
([8eadfe1](8eadfe1))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
ahmad-ibra pushed a commit that referenced this pull request Aug 1, 2024
🤖 I have created a release *beep* *boop*
---


##
[0.0.51](v0.0.50...v0.0.51)
(2024-07-31)


### Refactoring

* create root level HelmConfig struct which applies to all HelmReleases
([#356](#356))
([8eadfe1](8eadfe1))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
ahmad-ibra pushed a commit that referenced this pull request Aug 1, 2024
🤖 I have created a release *beep* *boop*
---


##
[0.0.51](v0.0.50...v0.0.51)
(2024-07-31)


### Refactoring

* create root level HelmConfig struct which applies to all HelmReleases
([#356](#356))
([8eadfe1](8eadfe1))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
ahmad-ibra added a commit that referenced this pull request Aug 1, 2024
…eleases (#356)

First step into resolving
- validator-labs/validatorctl#100

In an effort to simplify the TUI prompts in validatorctl to not require
lots of re-prompts for the helm configuration, we first need to refactor
our helm related structs.

The proposed structure would be this:

- `ValidatorConfigSpec.HelmConfig` defines the helm registry
configuration that would be used to pull all plugin charts
- `ValidatorConfigSpec.Plugins` has been simplifies to only specify the
charts repository, name, version, and values.

Along side this change, I updated the `reviewable-ext` make target to
also update the CRDs in the validator helm chart
This was referenced Aug 1, 2024
ahmad-ibra added a commit that referenced this pull request Aug 1, 2024
🤖 I have created a release *beep* *boop*
---


##
[0.1.0](v0.0.50...v0.1.0)
(2024-08-01)


### ⚠ BREAKING CHANGES

* create root level HelmConfig struct which applies to all HelmReleases
([#356](#356))

### Features

* create root level HelmConfig struct which applies to all HelmReleases
([#356](#356))
([6fe04ba](6fe04ba))


### Bug Fixes

* set oci.ImageOptions.Ref to the correct value to ensure plugin charts
are installed
([#359](#359))
([effd172](effd172))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: Ahmad Malik Ibrahim <ahmad.ibrahim@spectrocloud.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement to an existing feature lgtm This PR has been approved by a maintainer size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants