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

Allow to specify custom annotations for the service helm template #190

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

kuskoman
Copy link

No description provided.

@kuskoman kuskoman force-pushed the allow-to-specify-svc-annotations branch 2 times, most recently from 371a122 to b9bd895 Compare June 18, 2024 15:39
@jbourdale
Copy link

#sre

@vmercierfr
Copy link
Contributor

Thanks for this PR, sounds good to me. But Kubernetes service has not test yet, can I ask you to add the 2 following changes:

  1. Create configs/helm/tests/values/with_service.yaml that will be used to set an annotation in tests:
service:
  annotations:
    annotation1: value1
  1. Create configs/helm/tests/service_test.yaml to test the Kubernetes service
# yaml-language-server: $schema=https://raw.githubusercontent.com/helm-unittest/helm-unittest/main/schema/helm-testsuite.json
---
suite: grafana dashboard tests
templates:
  - service.yaml
tests:
  - it: render default service
    asserts:
      - isKind:
          of: Service
      - hasDocuments:
          count: 1
  - it: with custom service
    values:
      - ./values/with_service.yaml
    asserts:
      - equal:
          path: metadata.annotations.annotation1
          value: value1

With this configuration, we'll test in the CI that Kubernetes service return is valid according to the Kubernetes api.

@vmercierfr vmercierfr self-assigned this Jun 28, 2024
@vmercierfr vmercierfr added the enhancement New feature or request label Jun 28, 2024
@kuskoman
Copy link
Author

@vmercierfr wow, i haven't noticed the tests for helm chart. since it is MIT licensed repository it is definitely something i will be looking to "steal" into my OS repos 😄
i will update the code in a moment

@kuskoman kuskoman force-pushed the allow-to-specify-svc-annotations branch from cdf9cfd to c112544 Compare June 28, 2024 10:38
@kuskoman
Copy link
Author

i modified it, but i have trouble running unit tests on my local machine. i've installed the unittest plugin but it is throwing errors:
obraz

Copy link
Contributor

@vmercierfr vmercierfr left a comment

Choose a reason for hiding this comment

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

LGTM.

I can't say why it's failing with EOF error in your environment, but it's correct and I can run the test suite with success (using Helm 3.13 and 3.15).

@vmercierfr
Copy link
Contributor

vmercierfr commented Jun 28, 2024

In order to merge the PR, @kuskoman can you sign all your commits (git rebase HEAD~3 --signoff) ?

See Github Reference

@kuskoman kuskoman force-pushed the allow-to-specify-svc-annotations branch from c112544 to 11bd0de Compare June 28, 2024 12:58
@kuskoman
Copy link
Author

ahh, i signed 3rd part commits by mistake, lemme fix that

@kuskoman kuskoman force-pushed the allow-to-specify-svc-annotations branch 2 times, most recently from ecdd1af to d934789 Compare June 28, 2024 13:02
@kuskoman
Copy link
Author

@vmercierfr since i started from your current head with just a single commit, could you rerun the tests just to make sure? now the branch should be in the correct state

@vmercierfr
Copy link
Contributor

Tests are ok:

helm unittest configs/helm
2024/06/28 15:49:17 found symbolic link in path: /Users/vincent.mercier/Documents/github/kuskoman/prometheus-rds-exporter/configs/helm/LICENSE resolves to /Users/vincent.mercier/Documents/github/kuskoman/prometheus-rds-exporter/LICENSE. Contents of linked file included and used
2024/06/28 15:49:17 found symbolic link in path: /Users/vincent.mercier/Documents/github/kuskoman/prometheus-rds-exporter/configs/helm/grafana_dashboards resolves to /Users/vincent.mercier/Documents/github/kuskoman/prometheus-rds-exporter/configs/grafana/public. Contents of linked file included and used

### Chart [ prometheus-rds-exporter-chart ] configs/helm

 PASS  deployment tests	configs/helm/tests/deployment_test.yaml
 PASS  grafana dashboard tests	configs/helm/tests/grafanadashboard_test.yaml
 PASS  service tests	configs/helm/tests/service_test.yaml
 PASS  service account tests	configs/helm/tests/serviceaccount_test.yaml
 PASS  service monitor tests	configs/helm/tests/servicemonitor_test.yaml

Charts:      1 passed, 1 total
Test Suites: 5 passed, 5 total
Tests:       16 passed, 16 total
Snapshot:    0 passed, 0 total
Time:        85.865375ms

But git commits are not correctly signed; you should have a verified icon of your commits.
Can you check your commits signature?

@kuskoman kuskoman force-pushed the allow-to-specify-svc-annotations branch from 036033b to 1ff6c72 Compare July 1, 2024 08:15
Jakub Surdej added 2 commits July 1, 2024 10:15
Signed-off-by: Jakub Surdej <jakub.sudej@shift4.com>
Signed-off-by: Jakub Surdej <kubasurdej@gmail.com>
Signed-off-by: Jakub Surdej <jakub.sudej@shift4.com>
Signed-off-by: Jakub Surdej <kubasurdej@gmail.com>
@kuskoman kuskoman force-pushed the allow-to-specify-svc-annotations branch from 1ff6c72 to a97c640 Compare July 1, 2024 08:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants