Skip to content

Conversation

HenkvanDyk
Copy link
Contributor

@HenkvanDyk HenkvanDyk commented May 1, 2025

Summary of changes:
The base test connection which is created when doing a helm create is included in this chart.
While useful, it is not always required and optionally disabling it is useful.

Checklist:

  • I updated the artifacthub.io/changes annotation in Chart.yml according to the documentation
  • Optional: I updated in README.md the Helm Values

@jdetroyes
Copy link
Contributor

Hello @HenkvanDyk

Helm is only executing the tests manually via helm test not when installing the chart.
I don't think this is necessary or it is only for not rendering the template but I don't see cases.

@HenkvanDyk
Copy link
Contributor Author

HenkvanDyk commented May 2, 2025

My use case - we have linting checks which ensure that any resources which are created are tagged with specific pod labels.
I can ask my team to update the linting rules to skip any linting rules on resources with annotation "helm.sh/hook": test, or otherwise I'd like to include the podLabels in the same way they are created in other pod spec templates.

@jdetroyes
Copy link
Contributor

By podLabels you mean the deployments labels ? Common labels are already added to test.

metadata:
  name: "{{ include "ollama.fullname" . }}-test-connection"
  namespace: {{ include "ollama.namespace" . }}
  labels:
    {{- include "ollama.labels" . | nindent 4 }}

@HenkvanDyk
Copy link
Contributor Author

HenkvanDyk commented May 7, 2025

@jdetroyes correct, but they cannot be extended like they can in deployments.
I've added the ability now.

@jdetroyes
Copy link
Contributor

Hello @HenkvanDyk,

Can you fix the linter before merging?

Thanks!

@jdetroyes jdetroyes changed the title Test connection feat: allow disabling test connection May 17, 2025
@jdetroyes jdetroyes merged commit 83e75aa into otwld:main May 17, 2025
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants