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

O2-IMS Operator #844

Merged
merged 8 commits into from
Feb 13, 2025
Merged

O2-IMS Operator #844

merged 8 commits into from
Feb 13, 2025
+1,562 −0

Conversation

dkosteck
Copy link
Contributor

@dkosteck dkosteck commented Jan 31, 2025 •
edited
Loading

A PR creating a O2-IMS Operator, including related management code, controllers, utilities, testing code, and documentation.

closes: #759
closes: #757

Update 02-03-2025: Now relies on nephio-project/api#66
This API update is because of a previous outdated field in the CRD, to test this before it is merged one must use the link to the CRD from that commit in the curl step of the README.

@nephio-prow nephio-prow bot requested review from henderiw and liamfallon January 31, 2025 20:31
@nephio-prow Nephio PROW
Copy link
Contributor

nephio-prow bot commented Jan 31, 2025

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

@dkosteck dkosteck force-pushed the o2ims-operator branch 2 times, most recently from a81c6f0 to 9174461 Compare January 31, 2025 21:09
dkosteck and others added 2 commits January 31, 2025 23:48
@dkosteck @arora-sagar @vjayaramrh
O2-IMS Operator
0b81f01 
Co-authored-by: Sagar Arora <sagar.arora@openairinterface.org>
Co-authored-by: Vishwanath Jayaraman <vjayaram@redhat.com>
Signed-off-by: Daniel Kostecki <dkosteck@redhat.com>
Signed-off-by: Sagar Arora <sagar.arora@openairinterface.org>
Signed-off-by: Vishwanath Jayaraman <vjayaram@redhat.com>
@dkosteck @arora-sagar @vjayaramrh
Update O2-IMS Operator README for repo and paths
670c826 
Co-authored-by: Sagar Arora <sagar.arora@openairinterface.org>
Co-authored-by: Vishwanath Jayaraman <vjayaram@redhat.com>
Signed-off-by: Daniel Kostecki <dkosteck@redhat.com>
Signed-off-by: Vishwanath Jayaraman <vjayaram@redhat.com>
Signed-off-by: Sagar Arora <sagar.arora@openairinterface.org>
This was referenced Feb 3, 2025
Merged
Merged
@dkosteck @arora-sagar @vjayaramrh
Update O2-IMS Operator for updated CRD:
c052e16 
Changed provisionedResources to provisionedResourceSet (NOTE: This requires the updated CRD in the api repo)
Removed cluster constants from templateParameters, moved to constants in manager.py using kopf.Memo

Co-authored-by: Sagar Arora <sagar.arora@openairinterface.org>
Co-authored-by: Vishwanath Jayaraman <vjayaram@redhat.com>
Signed-off-by: Daniel Kostecki <dkosteck@redhat.com>
Signed-off-by: Vishwanath Jayaraman <vjayaram@redhat.com>
Signed-off-by: Sagar Arora <sagar.arora@openairinterface.org>
efiacor
efiacor reviewed Feb 4, 2025
View reviewed changes
@dkosteck @arora-sagar @vjayaramrh
Unit tests for utils.py:
e0f33e9 
Updates to README for unit tests
init to make controllers and tests modules
Updates to utils as a result of testing
test_utils PyTest unit tests, as well as accompanying requirements

Co-authored-by: Sagar Arora <sagar.arora@openairinterface.org>
Co-authored-by: Vishwanath Jayaraman <vjayaram@redhat.com>
@vjayaramrh vjayaramrh force-pushed the o2ims-operator branch 4 times, most recently from e7351b8 to 397a7c8 Compare February 11, 2025 00:21
vjayaramrh
vjayaramrh reviewed Feb 11, 2025
View reviewed changes
operators/o2ims-operator/README.md Outdated
sudo sysctl -w kernel.keys.maxkeys=500000
sudo sysctl -w kernel.keys.maxbytes=1000000
# Get the CRD from the Nephio API repo and place it in o2ims-operator/config/crd/bases/
curl --create-dirs -O --output-dir ./config/crd/bases/ https://raw.githubusercontent.com/nephio-project/api/refs/heads/main/config/crd/bases/o2ims.provisioning.oran.org_provisioningrequests.yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

@arora-sagar will modify L47 as below
kubectl create -f https://raw.githubusercontent.com/nephio-project/api/refs/heads/main/config/crd/bases/o2ims.provisioning.oran.org_provisioningrequests.yaml

Copy link
Contributor

Choose a reason for hiding this comment

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

I updated the text instead.

@vjayaramrh vjayaramrh force-pushed the o2ims-operator branch 2 times, most recently from cef1082 to 0affa2d Compare February 11, 2025 03:50
@vjayaramrh @dkosteck @arora-sagar
addressed code review comments
7949a46 
Co-authored-by: Daniel Kostecki <dkosteck@redhat.com>
Co-authored-by: Sagar Arora <sagar.arora@openairinterface.org>
Co-authored-by: Vishwanath Jayaraman <vjayaram@redhat.com>
@dkosteck dkosteck marked this pull request as ready for review February 11, 2025 15:33
@nephio-prow nephio-prow bot requested a review from johnbelamaric February 11, 2025 15:33
@arora-sagar arora-sagar removed the request for review from johnbelamaric February 12, 2025 17:30
vjayaramrh
vjayaramrh reviewed Feb 12, 2025
View reviewed changes
operators/o2ims-operator/README.md
Operator CRD is can be fetched via below command, though the above cluster creation script automatically fetches and apply this CRD.

```bash
curl --create-dirs -O --output-dir ./config/crd/bases/ https://raw.githubusercontent.com/nephio-project/api/refs/heads/main/config/crd/bases/o2ims.provisioning.oran.org_provisioningrequests.yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

@arora-sagar What is the reason for reverting this back?

Copy link
Contributor

Choose a reason for hiding this comment

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

@vjayaramrh This way the user can read it if the want, because else create-cluster.sh already deployed it anyways.

@arora-sagar @dkosteck @vjayaramrh
update python image and remove dead code
0b17ce6 
Co-authored-by: Daniel Kostecki <dkosteck@redhat.com>
Co-authored-by: Vishwanath Jayaraman <vjayaram@redhat.com>
Signed-off-by: Daniel Kostecki <dkosteck@redhat.com>
Signed-off-by: Vishwanath Jayaraman <vjayaram@redhat.com>
Signed-off-by: Sagar Arora <sagar.arora@openairinterface.org>
liamfallon
liamfallon reviewed Feb 13, 2025
View reviewed changes
Copy link
Member

@liamfallon liamfallon left a comment

Choose a reason for hiding this comment

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

/approve

@nephio-prow nephio-prow bot added the approved label Feb 13, 2025
@efiacor
Copy link
Collaborator

efiacor commented Feb 13, 2025 •
edited
Loading

It looks like the tests are now failing.

They will need to be updated due to removed dead code in this commit - 7949a46#diff-6312dad5e856554fe2dcd83141f2d9d696f54cec8bec4413f0eb206a220f9fd1

Also...
Could we drop in a tox.ini for the test setup? And update the requirements.txt.

# tox.ini

[tox]
# Define the environment you want to test. In this case, Python 3.11.
envlist = py311, lint

# Test environment for Python 3.11
[testenv:py311]
# Install dependencies necessary for running tests (e.g., pytest, pytest-cov)
deps =
    -rrequirements.txt
    pytest-cov
commands =
    pytest --maxfail=1 --disable-warnings -q

# Lint environment for running code style checks
[testenv:lint]
# Dependencies for linting with flake8
deps =
    flake8
commands =
    flake8 controllers/

This will help with hooking in a CI job later

@vjayaramrh @dkosteck @arora-sagar
removed unit test dead code
ad8f4fb 
Co-authored-by: Daniel Kostecki <dkosteck@redhat.com>
Co-authored-by: Vishwanath Jayaraman <vjayaram@redhat.com>
Co-authored-by: Sagar Arora <sagar.arora@openairinterface.org>
Signed-off-by: Daniel Kostecki <dkosteck@redhat.com>
Signed-off-by: Vishwanath Jayaraman <vjayaram@redhat.com>
Signed-off-by: Sagar Arora <sagar.arora@openairinterface.org>
@vjayaramrh
Copy link
Contributor

It looks like the tests are now failing.

They will need to be updated due to removed dead code in this commit - 7949a46#diff-6312dad5e856554fe2dcd83141f2d9d696f54cec8bec4413f0eb206a220f9fd1

Also... Could we drop in a tox.ini for the test setup? And update the requirements.txt.

# tox.ini

[tox]
# Define the environment you want to test. In this case, Python 3.11.
envlist = py311, lint

# Test environment for Python 3.11
[testenv:py311]
# Install dependencies necessary for running tests (e.g., pytest, pytest-cov)
deps =
    -rrequirements.txt
    pytest-cov
commands =
    pytest --maxfail=1 --disable-warnings -q

# Lint environment for running code style checks
[testenv:lint]
# Dependencies for linting with flake8
deps =
    flake8
commands =
    flake8 controllers/

This will help with hooking in a CI job later

@efiacor Could use some hand holding here as I am not familiar with tox

efiacor
efiacor reviewed Feb 13, 2025
View reviewed changes
@vjayaramrh @dkosteck @arora-sagar
added tox.ini file
647eaf0 
Co-authored-by: Daniel Kostecki <dkosteck@redhat.com>
Co-authored-by: Vishwanath Jayaraman <vjayaram@redhat.com>
Co-authored-by: Sagar Arora <sagar.arora@openairinterface.org>
Signed-off-by: Daniel Kostecki <dkosteck@redhat.com>
Signed-off-by: Vishwanath Jayaraman <vjayaram@redhat.com>
Signed-off-by: Sagar Arora <sagar.arora@openairinterface.org>
@efiacor
Copy link
Collaborator

efiacor commented Feb 13, 2025

/approve

@liamfallon
Copy link
Member

/approve
/lgtm

@nephio-prow nephio-prow bot added the lgtm label Feb 13, 2025
@nephio-prow Nephio PROW
Copy link
Contributor

nephio-prow bot commented Feb 13, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: efiacor, 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 7c58cb4 into nephio-project:main Feb 13, 2025
11 checks passed
nephio-prow bot pushed a commit that referenced this pull request Feb 14, 2025
@radoslawc
O2 IMS Operator dev image build (#845)
a5f4a67 
Builds dev image for: #844
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@radoslawc radoslawc radoslawc left review comments

@liamfallon liamfallon liamfallon left review comments

@efiacor efiacor efiacor left review comments

@vjayaramrh vjayaramrh vjayaramrh left review comments

@arora-sagar arora-sagar arora-sagar left review comments

@henderiw henderiw Awaiting requested review from henderiw

Assignees

@liamfallon liamfallon

Labels
approved lgtm
Projects
R4
Archived in project
Milestone
No milestone
6 participants
@dkosteck @efiacor @vjayaramrh @liamfallon @radoslawc @arora-sagar