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

fix lint errors #30

Closed
wants to merge 3 commits into from
Closed

fix lint errors #30

wants to merge 3 commits into from

Conversation

dacleyra
Copy link

[ERROR] templates/mancenter-deployment.yaml: invalid schema: ValidationError(Deployment.spec.template.spec.containers[0].securityContext): unknown field "fsGroup" in io.k8s.api.core.v1.SecurityContext

fsGroup is not a option for the securityContext of the Container, only the Pod

https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.12/#securitycontext-v1-core


[ERROR] templates/mancenter-deployment.yaml: missing required metadata labels: missing chart, heritage under spec.template.metadata.labels

Added the labels


[ERROR] templates/statefulset.yaml: invalid schema: [


ValidationError(StatefulSet.spec.template.spec.containers[0].securityContext): unknown field "fsGroup" in io.k8s.api.core.v1.SecurityContext,

fsGroup is not a option for the securityContext of the Container, only the Pod

https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.12/#securitycontext-v1-core


ValidationError(StatefulSet.spec.template.spec.containers[0]): unknown field "serviceAccountName" in io.k8s.api.core.v1.Container,

serviceAccountName is not an option on Container, only Pod

https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.12/#container-v1-core


ValidationError(StatefulSet.spec): missing required field "serviceName" in io.k8s.api.apps.v1.StatefulSetSpec]

Connect the StatefulSet to the Service identity

https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.12/#statefulsetspec-v1-apps
https://kubernetes.io/docs/concepts/workloads/controllers/statefulset/#stable-network-id

Copy link

@leszko leszko left a comment

Choose a reason for hiding this comment

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

I added 2 comments, please take a look. When we agree on the final version, could you also make the same changes to stable/hazelcast-enterprise?

@@ -83,11 +86,9 @@ spec:
{{- end }}
- name: JAVA_OPTS
value: "-Dhazelcast.rest.enabled={{ .Values.hazelcast.rest }} -Dhazelcast.config=/data/hazelcast/hazelcast.xml -DserviceName={{ template "hazelcast.fullname" . }} -Dnamespace={{ .Release.Namespace }} -Dhazelcast.mancenter.enabled={{ .Values.mancenter.enabled }} -Dhazelcast.mancenter.url=http://{{ template "mancenter.fullname" . }}:{{ .Values.mancenter.service.port }}/hazelcast-mancenter {{ if .Values.gracefulShutdown.enabled }}-Dhazelcast.shutdownhook.policy=GRACEFUL -Dhazelcast.shutdownhook.enabled=true -Dhazelcast.graceful.shutdown.max.wait={{ .Values.gracefulShutdown.maxWaitSeconds }} {{ end }} {{ if .Values.metrics.enabled }}-Dhazelcast.jmx=true{{ end }} {{ .Values.hazelcast.javaOpts }}"
serviceAccountName: {{ template "hazelcast.serviceAccountName" . }}
Copy link

Choose a reason for hiding this comment

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

What service account it used if it's not specified here?

Copy link
Author

Choose a reason for hiding this comment

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

serviceAccountName is already being set at the Pod object level correctly here

serviceAccountName: {{ template "hazelcast.serviceAccountName" . }}

serviceAccountName is not valid at the Container object level

@@ -81,7 +83,6 @@ spec:
{{- if .Values.securityContext.enabled }}
securityContext:
runAsUser: {{ .Values.securityContext.runAsUser }}
fsGroup: {{ .Values.securityContext.fsGroup }}
Copy link

Choose a reason for hiding this comment

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

How do you set the fsGroup then? Don't we have any case when it's useful to set it?

Copy link
Author

Choose a reason for hiding this comment

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

fsGroup is already being set at the Pod securityContext object level correctly here

fsGroup: {{ .Values.securityContext.fsGroup }}

fsGroup is not valid at the Container securityContext object level

Copy link

@leszko leszko left a comment

Choose a reason for hiding this comment

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

Thanks @dacleyra .

  1. Could you make the same change to hazelcast-enterprise?
  2. Could you bump up the chart version (to 1.3.1)?

@dacleyra
Copy link
Author

@leszko

  • changeset made for hazelcast-enterprise
  • chart version set to 1.3.1

@mesutcelik
Copy link

@dacleyra
Can you please rebase?

@dbrimley
Copy link

Any updates on this, be good to get this in before we push to the main helm repo.

@dacleyra
Copy link
Author

I have not gotten approval to sign the CLA yet, sorry.

@dacleyra
Copy link
Author

I was denied permission to sign the CLA.
closing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants