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

Add metrics.yaml to ran-simulator #248

Merged
merged 1 commit into from
Mar 3, 2021

Conversation

MatthewWEdwards
Copy link
Contributor

No description provided.

@@ -0,0 +1,20 @@
startup:
rc:
Copy link
Contributor

Choose a reason for hiding this comment

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

reuse the file in the metrics. Perhaps we should name it metrics.yaml rather than startup.yaml.

Copy link
Contributor

Choose a reason for hiding this comment

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

Top level model is also startup. so it makes more sense to be specefic.

earfcn: 7123
pci: 42
pciPool:
- min: 40
Copy link
Contributor

Choose a reason for hiding this comment

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

You also need to increase chart version. and after ransim merged. we need to release it and update app version as well.

@MatthewWEdwards MatthewWEdwards changed the title [WIP] [Do Not Merge] Add startup.yaml to ran-simulator Add metrics.yaml to ran-simulator Feb 26, 2021
@@ -60,6 +60,7 @@ spec:
- name: GODEBUG
value: cgocheck=0
args:
#- "100000"
Copy link
Contributor

Choose a reason for hiding this comment

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

what is this for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was accidentally committed, removed.

@@ -0,0 +1,20 @@
metrics:
Copy link
Contributor

Choose a reason for hiding this comment

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

We already have a file there. Please use the same file in terms of content. https://github.com/onosproject/sdran-helm-charts/blob/master/ran-simulator/files/metrics/pci.yaml

Copy link
Contributor

Choose a reason for hiding this comment

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

Also rebse your local repo and resolve conflicts

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copied the content from pci.yaml to metrics.yaml and removed pci.yaml

@MatthewWEdwards
Copy link
Contributor Author

  • Rebase
  • Update metrics.yaml content
  • Remove pci.yaml template and file
  • Remove changes to deployment.yaml

@@ -16,7 +16,7 @@ replicaCount: 1
image:
registry: ""
repository: onosproject/ran-simulator
tag: v0.7.10
tag: v0.7.11
Copy link
Contributor

Choose a reason for hiding this comment

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

This will be increased when we release the ransim. You need to increase chart version for each release.

Copy link
Contributor Author

@MatthewWEdwards MatthewWEdwards Feb 26, 2021

Choose a reason for hiding this comment

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

Okay, so should I revert this diff back to v0.7.10? I'm a little confused what you meant by #248 (comment). Do I need to create a new PR when you release ransim v0.7.11? Do I need to take any action to update the tag in the ransim helm chart or is that done automatically?

Copy link
Contributor

Choose a reason for hiding this comment

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

It is ok to increase the tag but we should merge ransim PR first and then release ransim image and then we can merge this PR. but what I don't see in your PR, the version of chart is not increased. look at chart.yaml. For each release of a chart you should increase chart version and that is why the build is failing.

Copy link
Contributor

Choose a reason for hiding this comment

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

@MatthewWEdwards
Copy link
Contributor Author

Bump chart version

@adibrastegarnia
Copy link
Contributor

I will try both of your PRs and then we merge.

@@ -7,7 +7,7 @@ name: ran-simulator
description: ONOS RAN Simulator
kubeVersion: ">=1.15.0"
type: application
version: 1.0.21
version: 1.0.22
appVersion: v0.7.10
Copy link
Contributor

Choose a reason for hiding this comment

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

@MatthewWEdwards Would you please update app version to v0.7.11?

- Remove pci.yaml file and the mapping from templates

- Add metrics.yaml file and mapping in templates

- Bump ran-simulator version in values.yaml
@adibrastegarnia adibrastegarnia merged commit 0de5fc4 into onosproject:master Mar 3, 2021
@adibrastegarnia adibrastegarnia deleted the patched branch March 3, 2021 18:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants