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.

Sign up for GitHub

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

Jump to bottom

Convert Nginx App to use "new" pkg/apps/helm_app style #222

Closed
Waterdrips opened this issue Oct 12, 2020 · 4 comments · Fixed by #224
Closed

Convert Nginx App to use "new" pkg/apps/helm_app style #222

Waterdrips opened this issue Oct 12, 2020 · 4 comments · Fixed by #224
Assignees
@nitishkumar71
Labels
good first issue Good for newcomers help wanted Extra attention is needed proposal skill/beginner

Comments

@Waterdrips
Copy link
Contributor

We need to move apps over to using the new app style.

We should migrate Nginx app over
https://github.com/alexellis/arkade/blob/master/cmd/apps/nginx_app.go
to use the style in https://github.com/alexellis/arkade/blob/master/pkg/apps/chart_app.go as per https://github.com/alexellis/arkade/blob/master/cmd/apps/loki_app.go

This will mean moving over to using this pattern https://github.com/alexellis/arkade/blob/master/cmd/apps/loki_app.go#L71-L76
of building the DefaultInstallOptions and setting overrides rather than doing all the helm install logic inside the app.

Expected Behaviour

This should be a re-factor. All existing functionality should remain the same.

Current Behaviour

ingress-nginx should be installed as now
@Waterdrips
Copy link
Contributor Author

/add label: proposal, help wanted, skill/beginner, good-first-issue

@Waterdrips
Copy link
Contributor Author

/remove label: good-first-issue

@derek derek bot removed the good-first-issue label Oct 12, 2020
@Waterdrips
Copy link
Contributor Author

/add label: good first issue

@derek derek bot added the good first issue Good for newcomers label Oct 12, 2020
@Waterdrips
Copy link
Contributor Author

/assign: nitishkumar71

nitishkumar71 added a commit to nitishkumar71/arkade that referenced this issue Oct 12, 2020
@nitishkumar71
convert nginx app to use new app style alexellis#222
3ba26b7 
Signed-off-by: Nitishkumar Singh <nitishkumarsingh71@gmail.com>
@nitishkumar71 nitishkumar71 mentioned this issue Oct 12, 2020
Merged
12 tasks
nitishkumar71 added a commit to nitishkumar71/arkade that referenced this issue Oct 13, 2020
@nitishkumar71
convert nginx app to use new app style alexellis#222
e41006a 
Signed-off-by: Nitishkumar Singh <nitishkumarsingh71@gmail.com>

included wait option and removed duplicated line

Signed-off-by: Nitishkumar Singh <nitishkumarsingh71@gmail.com>
alexellis pushed a commit that referenced this issue Oct 13, 2020
@nitishkumar71 @alexellis
convert nginx app to use new app style #222
479e2f1 
Signed-off-by: Nitishkumar Singh <nitishkumarsingh71@gmail.com>

included wait option and removed duplicated line

Signed-off-by: Nitishkumar Singh <nitishkumarsingh71@gmail.com>
@nitishkumar71 nitishkumar71 mentioned this issue Oct 24, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@nitishkumar71 nitishkumar71

Labels
good first issue Good for newcomers help wanted Extra attention is needed proposal skill/beginner
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Convert ingress-nginx app to use new app style #222 nitishkumar71/arkade
2 participants
@nitishkumar71 @Waterdrips