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

Add missing flag to Install #71

Merged
merged 1 commit into from
Mar 9, 2021

Conversation

Itxaka
Copy link
Contributor

@Itxaka Itxaka commented Mar 8, 2021

When mimicking the addInstallFlags from Install we
missed the values flags and the chartoptions flags

Fixes: #68

Signed-off-by: Itxaka [email protected]

@Itxaka Itxaka added bug Something isn't working ready for review Somebody please take a look at this and review labels Mar 8, 2021
@Itxaka
Copy link
Contributor Author

Itxaka commented Mar 8, 2021

license check failing due to #72

@kkaempf
Copy link
Collaborator

kkaempf commented Mar 8, 2021

Is there a better way to implement this ? We would need to update hypper every time helm adds a new flag. Can't we just pass all (unknown) flags to helm ?

@Itxaka
Copy link
Contributor Author

Itxaka commented Mar 8, 2021

Is there a better way to implement this ? We would need to update hypper every time helm adds a new flag. Can't we just pass all (unknown) flags to helm ?

On Upgrade we are using the helm flags directly without modifying them in our side. In this case I think its because we need to add a flag that collides with the flag passed from Helm (Due to hypper skipping the release name as a feature)

I think we should open another ticket to deal with the flag stuff as we may be able to bring Helm flags and then force-override ours on top somehow.

@dragonchaser
Copy link
Contributor

Is there a better way to implement this ? We would need to update hypper every time helm adds a new flag. Can't we just pass all (unknown) flags to helm ?

I don't think this is feasible, we also might include changes to our own values in the annotated charts, so we have to evaluate them anyhow.

When mimicking the addInstallFlags from Install we
missed the values flags and the chartoptions flags

Signed-off-by: Itxaka <[email protected]>
@Itxaka Itxaka force-pushed the fix_values_missing_install branch from 0339c5b to 14b024b Compare March 9, 2021 08:46
@viccuad
Copy link
Member

viccuad commented Mar 9, 2021

Thanks for looking into this :)! Good to have usage parity with Helm.

@Itxaka Itxaka merged commit 1a686aa into rancher-sandbox:main Mar 9, 2021
@Itxaka Itxaka deleted the fix_values_missing_install branch March 9, 2021 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working ready for review Somebody please take a look at this and review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

helm install option --values not recognized
4 participants