feat(orc8r): K8s Include labels and annotations based on the image/chart being deployed #10085

joary · 2021-11-04T02:44:02Z

Summary

In essence the same as #9981, adding two points:

The labels also to pods/containers
The chart-version as a new label to both lte-orc8r and orc8r charts.

More details:

Include a label in the orc8r and lte-orc8r charts including the current version being deployed
1. The label value comes directly from the docker image tag, which is the closes information the k8s have to the magma version
Include an annotation in the orc8r and lte-orc8r charts including the cluster release name being deploied (production/staging/green/blue)
1. Annotation values comes from the helm chart name being deployed (configurable in terraform values)
OBS: For bot annotation and labels some K8s objects does not include this label since they’re independent of the magma version being run. Non exaustive list: Secrets/ConfigMaps/Certs/Roles/RoleBindings

Also fixed some bugs to allow testing:

Avoid to running metrics storeconfig job if prometheus is not enabled
Avoid creating PodDisruptionBudgets if not enabled

Test Plan

A - Check the labels / annotations for each k8s object with helm template:

Result for 'orc8r' helm chart: https://gist.github.com/joary/e7156e6f6fce6834323aaa709925f6b8

Result for 'lte-orc8r' helm chart:
https://gist.github.com/joary/aa3a8bf3974bc5ec9ca98ed35cd57325

B - Check the labels for each pod after deploying both orc8r and lte-orc8r charts

kubectl get pods -n <namespace> --show-labels:

https://gist.github.com/joary/c95906e247634f21f277d85772222c42

Additional Information

This change is backwards-breaking

github-actions · 2021-11-04T02:44:18Z

Thanks for opening a PR! 💯

A couple initial guidelines

All commits must be signed off. This is enforced by DCO check.
All PR titles must follow the semantic commits format. This is enforced by Semantic PR.

Howto

Reviews. The "Reviewers" listed for this PR are the Magma maintainers who will shepherd it.
Checks. All required CI checks must pass before merge.
Merge. Once approved and passing CI checks, use the ready2merge label to indicate the maintainers can merge your PR.

More info

Please take a moment to read through the Magma project's

Contributing Conventions for norms around contributed code

If this is your first Magma PR, also consider reading

Developer Onboarding for onboarding as a new Magma developer
Development Workflow for guidance on your first pull request
CI Checks for points of contact for failing or flaky CI checks
GitHub-to-Slack mappings for Magma maintainers for guidance on how to contact maintainers on Slack

github-actions · 2021-11-04T02:44:58Z

nms-workflow

0 files 0 suites 0s ⏱️
0 tests 0 ✔️ 0 💤 0 ❌

Results for commit 89f9715.

♻️ This comment has been updated with latest results.

github-actions · 2021-11-04T02:51:59Z

feg-workflow

    2 files 199 suites 32s ⏱️
350 tests 350 ✔️ 0 💤 0 ❌
364 runs 364 ✔️ 0 💤 0 ❌

Results for commit f5116ed.

♻️ This comment has been updated with latest results.

github-actions · 2021-11-04T03:00:34Z

cloud-workflow

    8 files 350 suites 2m 29s ⏱️
833 tests 833 ✔️ 0 💤 0 ❌

Results for commit f5116ed.

♻️ This comment has been updated with latest results.

github-actions · 2021-11-04T03:24:16Z

agw-workflow

  28 files   43 suites 3m 37s ⏱️
685 tests 676 ✔️ 9 💤 0 ❌
686 runs 677 ✔️ 9 💤 0 ❌

Results for commit f5116ed.

♻️ This comment has been updated with latest results.

hcgatewood

Looks like this is still stacked on the previous PR (?). Can you rebase it onto latest master then re-request a review?

arunuke

A general comment is to just check if all the pods we deploy are tagged with the image and chart versions as desired.

arunuke · 2021-11-12T09:43:32Z

orc8r/cloud/helm/orc8r/charts/nms/templates/nginx-proxy-deployment.yaml

  template:
    metadata:
      labels:
 {{ tuple $envAll "nms" "nginx" | include "nms.selector-labels" | indent 8 }}
+{{ include "magmalte-image-version-label" . | indent 8 }}


Should we use magmalte-image-version-label for nginx-proxy deployment as well? Looking at our artifactory for helm charts, we have one for orc8r and one for lte-orc8r which I presume handles it for the controller and magmalte images respectively. If so, is there a possibility that nginx-proxy and magmalte could have different chart versions?

Should we use magmalte-image-version-label for nginx-proxy deployment as well

No, I think we should use the nginx-image-version-label. I corrected the PR to reflect that.

If so, is there a possibility that nginx-proxy and magmalte could have different chart versions?

No, both nginx deployments will use the same helm value configuration.

Being more clear, there are two nginx deployments:

One handling the public access to NMS

Another handling the public access to Orc8r services

We expect them to have the same image-label value, though the char-version will be different, #1 will receive the NMS sub-chart version, and the #2 will receive Orc8r version.

arunuke

Thanks for the changes. LGTM.

I remember we had a plan to upgrade the EKS version as well (due to future support for nginx ingress). Is it going in a different PR?

joary · 2021-11-30T12:35:33Z

I remember we had a plan to upgrade the EKS version as well (due to future support for nginx ingress). Is it going in a different PR?

Yes, I included in #10212, which is merged now. Rationale is that PR changes the k8s resources already, made more sense to be there.

hcgatewood

Skimmed, lgtm

…etup Assuming we're not deploying prometheus these configurations will not be needed. In addition if the job fails it will not allow other services to be deployed. Signed-off-by: Joary Paulo Wanzeler Fortuna <joarypl@fb.com>

This applies the same behaviour defined in orc8rlib in templates/_pdb.yaml Signed-off-by: Joary Paulo Wanzeler Fortuna <joarypl@fb.com>

…being deployied Signed-off-by: Joary Paulo Wanzeler Fortuna <joarypl@fb.com>

…c8r charts Also include labels to pods template Signed-off-by: Joary Paulo Wanzeler Fortuna <joarypl@fb.com>

github-actions · 2021-12-02T20:57:32Z

Unit Test Results

  28 files   28 suites 45m 49s ⏱️
176 tests 174 ✔️ 1 💤 0 ❌ 1 🔥

For more details on these errors, see this check.

Results for commit 728ceae.

♻️ This comment has been updated with latest results.

joary requested review from a team and hcgatewood November 4, 2021 02:44

pull-request-size bot added the size/L Denotes a Pull Request that changes 100-499 lines. label Nov 4, 2021

github-actions bot added component: orc8r Orchestrator-related issue and removed size/L Denotes a Pull Request that changes 100-499 lines. labels Nov 4, 2021

hcgatewood reviewed Nov 4, 2021

View reviewed changes

joary linked an issue Nov 5, 2021 that may be closed by this pull request

Orc8r Pod Labels #10103

Closed

joary force-pushed the helm-chart-labels-n-annotations branch from f6c3bb7 to 89f9715 Compare November 9, 2021 13:21

pull-request-size bot added the size/L Denotes a Pull Request that changes 100-499 lines. label Nov 9, 2021

github-actions bot removed the size/L Denotes a Pull Request that changes 100-499 lines. label Nov 9, 2021

joary requested review from arunuke and hcgatewood November 9, 2021 13:22

arunuke suggested changes Nov 12, 2021

View reviewed changes

joary force-pushed the helm-chart-labels-n-annotations branch from 89f9715 to eb0d164 Compare November 26, 2021 16:49

pull-request-size bot added the size/L Denotes a Pull Request that changes 100-499 lines. label Nov 26, 2021

joary force-pushed the helm-chart-labels-n-annotations branch from eb0d164 to 0aed2c3 Compare November 26, 2021 17:12

joary requested a review from arunuke November 26, 2021 17:15

arunuke approved these changes Nov 30, 2021

View reviewed changes

hcgatewood approved these changes Dec 1, 2021

View reviewed changes

joary added 4 commits December 2, 2021 14:19

fix(orc8r): Avoid creating PodDisruptionBudgets if not enabled or valid

a54a6b8

This applies the same behaviour defined in orc8rlib in templates/_pdb.yaml Signed-off-by: Joary Paulo Wanzeler Fortuna <joarypl@fb.com>

feat(orc8r): Include labels and annotations based on the image/chart …

170a357

…being deployied Signed-off-by: Joary Paulo Wanzeler Fortuna <joarypl@fb.com>

feat(orc8r): Include chart version in the labels for orc8r and lte-or…

f5116ed

…c8r charts Also include labels to pods template Signed-off-by: Joary Paulo Wanzeler Fortuna <joarypl@fb.com>

joary force-pushed the helm-chart-labels-n-annotations branch from 0aed2c3 to f5116ed Compare December 2, 2021 17:19

arunuke merged commit 728ceae into magma:master Dec 2, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(orc8r): K8s Include labels and annotations based on the image/chart being deployed #10085

feat(orc8r): K8s Include labels and annotations based on the image/chart being deployed #10085

joary commented Nov 4, 2021

github-actions bot commented Nov 4, 2021

github-actions bot commented Nov 4, 2021 •

edited

Loading

github-actions bot commented Nov 4, 2021 •

edited

Loading

github-actions bot commented Nov 4, 2021 •

edited

Loading

github-actions bot commented Nov 4, 2021 •

edited

Loading

hcgatewood left a comment

arunuke left a comment

arunuke Nov 12, 2021

joary Nov 26, 2021

arunuke left a comment

joary commented Nov 30, 2021 •

edited

Loading

hcgatewood left a comment

github-actions bot commented Dec 2, 2021 •

edited

Loading

feat(orc8r): K8s Include labels and annotations based on the image/chart being deployed #10085

feat(orc8r): K8s Include labels and annotations based on the image/chart being deployed #10085

Conversation

joary commented Nov 4, 2021

Summary

Test Plan

Additional Information

github-actions bot commented Nov 4, 2021

Howto

More info

github-actions bot commented Nov 4, 2021 • edited Loading

nms-workflow

github-actions bot commented Nov 4, 2021 • edited Loading

feg-workflow

github-actions bot commented Nov 4, 2021 • edited Loading

cloud-workflow

github-actions bot commented Nov 4, 2021 • edited Loading

agw-workflow

hcgatewood left a comment

Choose a reason for hiding this comment

arunuke left a comment

Choose a reason for hiding this comment

arunuke Nov 12, 2021

Choose a reason for hiding this comment

joary Nov 26, 2021

Choose a reason for hiding this comment

arunuke left a comment

Choose a reason for hiding this comment

joary commented Nov 30, 2021 • edited Loading

hcgatewood left a comment

Choose a reason for hiding this comment

github-actions bot commented Dec 2, 2021 • edited Loading

Unit Test Results

github-actions bot commented Nov 4, 2021 •

edited

Loading

github-actions bot commented Nov 4, 2021 •

edited

Loading

github-actions bot commented Nov 4, 2021 •

edited

Loading

github-actions bot commented Nov 4, 2021 •

edited

Loading

joary commented Nov 30, 2021 •

edited

Loading

github-actions bot commented Dec 2, 2021 •

edited

Loading