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

Resources & Annotations: Map or String? #292

Closed
issacg opened this issue May 6, 2020 · 7 comments
Closed

Resources & Annotations: Map or String? #292

issacg opened this issue May 6, 2020 · 7 comments
Assignees
Labels
bug Something isn't working docs Improvements or additions to documentation

Comments

@issacg
Copy link

issacg commented May 6, 2020

According to the docs resources and annotations should be strings.

According to the defaults in values.yaml it seems that both should be maps. Indeed trying to follow the docs gives me the following output:

coalesce.go:196: warning: cannot overwrite table with non table for resources (map[])
coalesce.go:196: warning: cannot overwrite table with non table for annotations (map[])
Error: unable to build kubernetes objects from release manifest: error validating "": error validating data: ValidationError(Deployment.spec.template.spec.containers[0].resources): invalid type for io.k8s.api.core.v1.ResourceRequirements: got "string", expected "map"

Are the docs wrong (outdated)? Or am I possibly doing something wrong?

I have an overrides file with snippets like these:

injector:
  resources: |
    requests:
      memory: 64Mi
      cpu: 10m
    limits:
      memory: 256Mi
      cpu: 250m

server:
  ingress:
    annotations: |
      kubernetes.io/ingress.class: traefik
      traefik.ingress.kubernetes.io/router.entrypoints: vault
@pcman312 pcman312 added bug Something isn't working docs Improvements or additions to documentation labels May 7, 2020
@pcman312
Copy link
Contributor

pcman312 commented May 7, 2020

Thanks for the issue! It looks like our documentation is incorrect. resources is a map type and annotations is a multi-line string at the moment, but we're looking into supporting both multi-line strings or a YAML map.

@oliviermichaelis
Copy link

oliviermichaelis commented May 7, 2020

I can confirm that removing the | works, but I still get the following warning:

coalesce.go:195: warning: destination for resources is a table. Ignoring non-table value <nil>

How do we get rid of the warning?

My overriding file looks like this:

server:
  resources:
    requests:
      memory: 256Mi
      cpu: 250m
    limits:
      memory: 256Mi
      cpu: 250m

and it templates out to this with helm template:

containers:
  - name: vault
    resources:
      limits:
        cpu: 250m
        memory: 256Mi
      requests:
        cpu: 250m
        memory: 256Mi

@tvoran
Copy link
Member

tvoran commented May 18, 2020

@issacg the docs for resources have been corrected.

@oliviermichaelis that warning is because the default for server.resources is blank in values.yaml, which defaults to a string, so we just need to update the default to be {}. But for now it should be safe to ignore the warning.

@tvoran
Copy link
Member

tvoran commented May 20, 2020

Resolved by #309 and hashicorp/vault#8973

@tvoran tvoran closed this as completed May 20, 2020
@maurya-m
Copy link

maurya-m commented May 29, 2020

@tvoran has this being merged? can you please help here?

i am still failing to install using the default values.yaml:

resources: {}
  # resources:
  #   requests:
  #     memory: 256Mi
  #     cpu: 250m
  #   limits:
  #     memory: 256Mi
  #     cpu: 250m

coalesce.go:195: warning: destination for resources is a table. Ignoring non-table value

NAME NAMESPACE REVISION UPDATED STATUS CHART APP VERSION
vault xxxx 1 2020-05-29 03:29:26.238520353 -0700 MST failed vault-0.5.0[](url)

@tvoran
Copy link
Member

tvoran commented May 29, 2020

Hi @maurya-m, yes it's been merged into master. That warning shouldn't be preventing the chart from installing, so I think something else must be going on here. The output from helm install should have more information on why it failed, otherwise try the --dry-run and --debug flags with helm install to get more information.

@maurya-m
Copy link

maurya-m commented Jun 1, 2020

@tvoran , thanks for replying here! I was able to deploy, the vault with now with global with false value:
global: enabled: true

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working docs Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

5 participants