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

Add prelim helm integration guide #66

Merged
merged 2 commits into from
Sep 22, 2023
Merged

Add prelim helm integration guide #66

merged 2 commits into from
Sep 22, 2023

Conversation

efiacor
Copy link
Collaborator

@efiacor efiacor commented Sep 20, 2023

Adding a placeholder for helm int docs and including a basic readme howto guide on using the existing flux kpt packages.
Also, some refactoring on the structure of the the docs repo.

@efiacor
Copy link
Collaborator Author

efiacor commented Sep 20, 2023

/cc @liamfallon

@nephio-prow nephio-prow bot requested a review from liamfallon September 20, 2023 18:37
@efiacor
Copy link
Collaborator Author

efiacor commented Sep 20, 2023

/cc @lapentad

@nephio-prow nephio-prow bot requested a review from lapentad September 20, 2023 18:38
@efiacor
Copy link
Collaborator Author

efiacor commented Sep 20, 2023

/cc @tliron

@nephio-prow nephio-prow bot requested a review from tliron September 20, 2023 18:38
@efiacor
Copy link
Collaborator Author

efiacor commented Sep 20, 2023

/cc @s3wong

@nephio-prow nephio-prow bot requested a review from s3wong September 20, 2023 18:38
@efiacor
Copy link
Collaborator Author

efiacor commented Sep 20, 2023

/cc @byungwoojun

@nephio-prow nephio-prow bot requested a review from byungwoojun September 20, 2023 18:38
@efiacor efiacor changed the title Adding prelim helm integration guide Add prelim helm integration guide Sep 21, 2023
@liamfallon
Copy link
Member

/approve

johnbelamaric
johnbelamaric requested changes Sep 22, 2023
View reviewed changes
Copy link
Member

@johnbelamaric johnbelamaric left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few minor tweaks, and I haven't tried it, but looks good generally.

user-guide/docs/helm/README.md Outdated
## Overview

Helm integration support in Nephio is currently under investigation
and will progress over the coming releases.\
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extraneous backslash

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed

user-guide/docs/helm/README.md Outdated

### Deployment options

[Using Flux CD to deploy helm charts in Nephio](https://github.com/nephio-project/docs/blob/main/install-guide/docs/helm/flux-helm.md)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make this a bulleted list, and then add the other options under consideration (even though they will not have links). At the very least add the Operator SDK and a link to that.

I would also change the link text to just "Using Flux CD".

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

user-guide/docs/helm/flux-helm.md Outdated
@@ -0,0 +1,143 @@
# Deploying helm charts in Nephio using Flux CD

[Flux CD](https://fluxcd.io/flux/use-cases/helm/) provides a set of kubernetes controllers to enable a GitOps driven deployment of helm charts, using relevant the flux Custom Resources.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/kubernetes/Kubernetes/

I would just end the sentence where the comma is.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@efiacor
Address review comments
267a303 
Signed-off-by: efiacor <fiachra.corcoran@est.tech>
johnbelamaric
johnbelamaric approved these changes Sep 22, 2023
View reviewed changes
Copy link
Member

@johnbelamaric johnbelamaric left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/approve
/lgtm

@nephio-prow
Copy link
Contributor

nephio-prow bot commented Sep 22, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: johnbelamaric, liamfallon

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@nephio-prow nephio-prow bot merged commit c792ef2 into nephio-project:main Sep 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@johnbelamaric johnbelamaric johnbelamaric approved these changes

@henderiw henderiw Awaiting requested review from henderiw

@liamfallon liamfallon Awaiting requested review from liamfallon

@lapentad lapentad Awaiting requested review from lapentad

@tliron tliron Awaiting requested review from tliron

@s3wong s3wong Awaiting requested review from s3wong

@byungwoojun byungwoojun Awaiting requested review from byungwoojun

Assignees

@johnbelamaric johnbelamaric

Labels
approved lgtm
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@efiacor @liamfallon @johnbelamaric