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

Updating docs for R2 #85

Merged
merged 4 commits into from
Jan 29, 2024
Merged

Updating docs for R2 #85

merged 4 commits into from
Jan 29, 2024

Conversation

arora-sagar
Copy link
Contributor

Updating documentation for release 2

  • Installation guide for GCE
  • Exercises for OAI and Free5GC

- Installation guide for GCE
- Exercises for OAI and Free5GC
Copy link
Contributor

nephio-prow bot commented Jan 18, 2024

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@arora-sagar arora-sagar marked this pull request as ready for review January 25, 2024 23:16
Copy link
Contributor

@efiacor efiacor left a comment

Choose a reason for hiding this comment

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

Hi @arora-sagar,
Looks good. My only concern is that there is a lot or duplication here. The main diff I see is the cluster provisioning and the nf deploy. I think the main purpose of the orig guide was to show the diff ways to deploy a pkg. ie kpt vs PV/PVS vs UI. I think we should keep that common to both use cases. Maybe in a separate section/file?

@arora-sagar
Copy link
Contributor Author

Hi @arora-sagar, Looks good. My only concern is that there is a lot or duplication here. The main diff I see is the cluster provisioning and the nf deploy. I think the main purpose of the orig guide was to show the diff ways to deploy a pkg. ie kpt vs PV/PVS vs UI. I think we should keep that common to both use cases. Maybe in a separate section/file?

@efiacor I don't really get it how you want to do that?

@efiacor
Copy link
Contributor

efiacor commented Jan 26, 2024

Hi @arora-sagar, Looks good. My only concern is that there is a lot or duplication here. The main diff I see is the cluster provisioning and the nf deploy. I think the main purpose of the orig guide was to show the diff ways to deploy a pkg. ie kpt vs PV/PVS vs UI. I think we should keep that common to both use cases. Maybe in a separate section/file?

@efiacor I don't really get it how you want to do that?

We prob should break some of the exercise steps into their own sections/files and reuse them. Say for instance the "inter-cluster networking" is common to both.

content/en/docs/guides/user-guides/exercise-2-oai.md Outdated Show resolved Hide resolved

## Introduction

Be sure you have followed the [installation guide]({{< ref "docs/guides/install-guides ">}})
Copy link
Contributor

Choose a reason for hiding this comment

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

Make sure to render/preview the links etc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

About the links I am not sure it is like this in the current exercise.md in main too. @CsatariGergely is it suppose to be like this?

content/en/docs/guides/user-guides/exercise-2-oai.md Outdated Show resolved Hide resolved
@arora-sagar
Copy link
Contributor Author

Hi @arora-sagar, Looks good. My only concern is that there is a lot or duplication here. The main diff I see is the cluster provisioning and the nf deploy. I think the main purpose of the orig guide was to show the diff ways to deploy a pkg. ie kpt vs PV/PVS vs UI. I think we should keep that common to both use cases. Maybe in a separate section/file?

@efiacor I don't really get it how you want to do that?

We prob should break some of the exercise steps into their own sections/files and reuse them. Say for instance the "inter-cluster networking" is common to both.

@efiacor Actually that section is not common either. Making the workload cluster in both of them is different the topology is different and the networking is also different.

Both exercises don't share the network, network topology right now is in dedicated sub-folders. That is the reason I am using the export commands to use the right path.

@efiacor
Copy link
Contributor

efiacor commented Jan 26, 2024

Hi @arora-sagar, Looks good. My only concern is that there is a lot or duplication here. The main diff I see is the cluster provisioning and the nf deploy. I think the main purpose of the orig guide was to show the diff ways to deploy a pkg. ie kpt vs PV/PVS vs UI. I think we should keep that common to both use cases. Maybe in a separate section/file?

@efiacor I don't really get it how you want to do that?

We prob should break some of the exercise steps into their own sections/files and reuse them. Say for instance the "inter-cluster networking" is common to both.

@efiacor Actually that section is not common either. Making the workload cluster in both of them is different the topology is different and the networking is also different.

Both exercises don't share the network, network topology right now is in dedicated sub-folders. That is the reason I am using the export commands to use the right path.

Ye maybe not the best example as the networking is all a bit hacky. But say for instance, these files are identical - https://github.com/nephio-project/test-infra/blob/main/e2e/tests/free5gc/clab-topo.gotmpl
https://github.com/nephio-project/test-infra/blob/main/e2e/tests/oai/clab-topo.gotmpl

We may need to revisit this later. I'll ignore the duplication for now.. 😄

@@ -100,9 +100,9 @@ sudo netplan apply
Log onto your VM and run the following command:

```bash
wget -O - https://raw.githubusercontent.com/nephio-project/test-infra/v1.0.1/e2e/provision/init.sh | \
wget -O - https://raw.githubusercontent.com/nephio-project/test-infra/v2.0.0/e2e/provision/init.sh | \
Copy link
Member

Choose a reason for hiding this comment

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

Also this one

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So the issue here is v2.0.0 tag doesn't exist. The documentation is released before the tag is released.

@liamfallon
Copy link
Member

@arora-sagar Cna you just check the links, it could be something with my setup that they are not working as I'm not using GCE so I'm just checking the links on my local machine.

@arora-sagar
Copy link
Contributor Author

The tag v2.0.0 tag mentioned in the document doesn't exist for the moment. The documentation is released before the tag is released.

@liamfallon
Copy link
Member

/lgtm
/approve

Copy link
Contributor

nephio-prow bot commented Jan 29, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: CsatariGergely, 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 added the approved label Jan 29, 2024
@nephio-prow nephio-prow bot merged commit 7365ed2 into nephio-project:main Jan 29, 2024
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants