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 UPF-P4 SW component #56

Merged
merged 76 commits into from
Nov 12, 2024
Merged

Add UPF-P4 SW component #56

merged 76 commits into from
Nov 12, 2024

Conversation

rafagarciaUMA
Copy link
Contributor

Add UPF-P4 SW component from my 'upf_p4_sw' branch to 'develop' branch.

@rafagarciaUMA
Added UPF-P4 SW component. WIP
2a4a32e
@rafagarciaUMA
Added UPF-P4 config files
beb5ee9
@rafagarciaUMA
Modified public.yaml file
3949213
@rafagarciaUMA
Changed name file to fix bug
f8c03e7
@rafagarciaUMA
Fixed name issue
957008c
@rafagarciaUMA
Fixed bug in upf_p4_install.yaml
a0e77df
@rafagarciaUMA
Changed mongodb-clients packet for mongosh packet for compatibility
6d3b470
@rafagarciaUMA
Fixed and Testing indentation
aaff4e6
@rafagarciaUMA
Testing
f3f4021
@rafagarciaUMA
More testing...
d69344c
@rafagarciaUMA
Changed struct upf_p4_install.yaml
4b24dd2
@rafagarciaUMA
Used include_task instead of include
fef4be4
@rafagarciaUMA
Fixed become permissions
f208a51
@rafagarciaUMA
Add become permission to install apk packets
7ba6b4b
@rafagarciaUMA
Removed mongosh installation
dab00c7
@rafagarciaUMA
Testing to ensure variable availability
f21d5da
@rafagarciaUMA
Testing variables availability
189df36
@rafagarciaUMA
More Testing
1ec5f08
@rafagarciaUMA
More Testing
1bd27f0
@rafagarciaUMA
More Testing...
4a0d35c
@rafagarciaUMA
Delegate facts true
1062d33
@rafagarciaUMA
Testing to send vars between stages with different hosts
5fd79e2
@rafagarciaUMA
Testing to fix synchronize issue
1b0efa2
@rafagarciaUMA
Changed module synchronize for copy
bd95d2e
@rafagarciaUMA
Testing..
55e408b
@rafagarciaUMA
Testing 2..
d016626
@rafagarciaUMA
Testing persist variables between different hosts
1b2837f
@rafagarciaUMA
Using hostvars and Testing
e4eab0d
@rafagarciaUMA
Changed permissions when config folder is copied
60dac2a
@rafagarciaUMA
Fixed bug in copy module
37321f6
@rafagarciaUMA
Fixing..
51f0c79
@rafagarciaUMA
Improve error handling when registering duplicate subscribers
76ed4dc
@rafagarciaUMA
Trying to fix one error
e5a05fc
@rafagarciaUMA
Added NAT task
852c1f4
@rafagarciaUMA
Fixed one minor error
d55232b
@rafagarciaUMA
Testing..
83b4fa1
@rafagarciaUMA
Remove NAT because not working
87d4c11
@rafagarciaUMA
Improved README
a174a6f
@rafagarciaUMA
Improved README..
3c7d883
@rafagarciaUMA
Updated README: Added how to add new UE...
034690e
@rafagarciaUMA
Minor update in README
35e511b
@rafagarciaUMA
Added INT-P4 SW component. WIP
4b24c60 
- Created initial component playbook structure with 4 stages:
  - Stage 1: Infrastructure as Code deployment
  - Stage 2: Component access preparation (TODO)
  - Stage 3: Configuration as Code preparation (TODO)
  - Stage 4: Results publication (TODO)

- Implemented Stage 1 with basic terraform workflow:
  - Loading environment variables
  - Preparing terraform working directory
  - Configuring VM resources
  - Executing terraform apply
@rafagarciaUMA
Remove previus changes
3226fda
@rafagarciaUMA
feat: Prepare UPF-P4 SW component for develop PR
a6f2f0d 
- Added new input parameters for UPF configuration
- Enhanced metadata dictionary and output handling
- Improved documentation and result templates
- Cleaned up unused code and comments
- Updated changelog for v0.3.0 release
@rafagarciaUMA
feat: Add Terraform output validation and Harbor token configuration
d7ac52c 
- Add validation for required Terraform output variables
- Configure Harbor token from site available components
- Remove commented disk configuration code
@rafagarciaUMA
fix: simplify harbor token reference in docker config template
9386ed4
@rafagarciaUMA
feat: uncomment output variables in UPF-P4 SW playbook
d7adb31 
Enables publishing of id, ips, enb_ipv4_n3 and dn_ipv4_n6 variables to TNLCM
@rafagarciaUMA
Remove base64 encoding from template variables and update metadata re…
088dcd2 
…ferences

- Comment out base64 encoding for ID, IPs and network addresses
- Update template to reference variables directly without output prefix
- Fix metadata references to use specific component variables
@rafagarciaUMA
feat: add proxy field to UPF-P4 metadata dictionary
595444c 
- Add proxy IP configuration (10.11.3.1) required for UERANSIM component
@rafagarciaUMA
docs: add logging instructions and debug mode configuration
62fdeac 
- Add detailed instructions for accessing container logs
- Document debug mode configuration in docker-compose
- Update README with step-by-step logging guide
@alvarocurt
Copy link
Member

Thank you so much Rafa.
Before merging, could you please remove the (autogenerated) files inside variables?
Those are just placeholder files from the dummy component to show where variables are loaded during the deployment, but have no real purpose in a functional component's code
Besides that, I think everything looks nice.

@alvarocurt
Copy link
Member

Additional comments:

  • I've seen some unencripted certificates published as blank files. Is that intentional? Isn't that a huge security flaw?
  • I would suggest adding the IPs and VM ID to the TNLCM callback. Process is really simple, you already load those variables during the main ansible playbook execution.
  • You have 2 extra task checking if the terraform outputs are correctly created. I never had to do that. Did you encounter any problems with that during development? I'm actually interested, in case those checks should be added to more components

alvarocurt
alvarocurt requested changes Nov 11, 2024
View reviewed changes
Copy link
Member

@alvarocurt alvarocurt left a comment

Choose a reason for hiding this comment

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

Review on comments

@rafagarciaUMA
feat: update component playbook metadata and remove unnecessary files.
a1b8083 
- Clean up autogenerated files from variables directory
- Add VM ID and IP information to component_playbook.yaml metadata
- Remove certificates, private keys and configuration files containing unnecessary data
@rafagarciaUMA
Copy link
Contributor Author

Thank you so much Alvaro for your thorough review! I've addressed all your comments:

  • I've removed all the autogenerated files from the variables directory
  • I've deleted the freeDiameter certificates. They were accidentally left over from open5gs 4G setup and are not needed for this component.
  • I've added the IPs and VM ID to the TNLCM callback as suggested.
  • Regarding the two extra tasks that check terraform outputs: I personally haven't experienced any issues with outputs in this component, but I had some problems with my other P4 telemetry component where I have different outputs. These tasks aren't strictly necessary really. I added them after reading about it as a "best practice" (thanks ChatGPT 😅)

@alvarocurt alvarocurt merged commit 15ef5e4 into develop Nov 12, 2024
@alvarocurt alvarocurt deleted the upf_p4_sw branch November 12, 2024 09:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alvarocurt alvarocurt alvarocurt approved these changes

@jmaciasportela jmaciasportela Awaiting requested review from jmaciasportela

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@rafagarciaUMA @alvarocurt