airflow-helm / charts

The User-Community Airflow Helm Chart is the standard way to deploy Apache Airflow on Kubernetes with Helm. Originally created in 2017, it has since helped thousands of companies create production-ready deployments of Airflow on Kubernetes.
https://github.com/airflow-helm/charts/tree/main/charts/airflow
Apache License 2.0
630 stars 474 forks source link

invalid pgbouncer-certs volume spec, fails resource validation #765

Closed enrique-ayala closed 10 months ago

enrique-ayala commented 11 months ago

Checks

Chart Version

8.7.1

Kubernetes Version

Client Version: version.Info{Major:"1", Minor:"24", GitVersion:"v1.24.3", GitCommit:"aef86a93758dc3cb2c658dd9657ab4ad4afc21cb", GitTreeState:"clean", BuildDate:"2022-07-13T14:30:46Z", GoVersion:"go1.18.3", Compiler:"gc", Platform:"windows/amd64"}
Kustomize Version: v4.5.4
Server Version: version.Info{Major:"1", Minor:"17", GitVersion:"v1.17.5", GitCommit:"e0fccafd69541e3750d460ba0f9743b90336f24f", GitTreeState:"clean", BuildDate:"2020-04-16T11:35:47Z", GoVersion:"go1.13.9", Compiler:"gc", Platform:"linux/amd64"}

Helm Version

version.BuildInfo{Version:"v3.2.1", GitCommit:"fe51cd1e31e6a202cba7dead9552a6d418ded79a", GitTreeState:"clean", GoVersion:"go1.13.10"}

Description

Hi good day, I'm upgrading chart version from 8.6.1 to 8.7.1 and received the following error:

│ Error: error validating "": error validating data: ValidationError(Deployment.spec.template.spec.volumes[1].projected): missing required field "sources" in io.k8s.api.core.v1.ProjectedVolumeSource

I have executed helm template to render values and compare . I can see the following for file # Source: airflow/templates/pgbouncer/pgbouncer-deployment.yaml

With 8.6.1, volume pgbouncer-certs looks good :

        - name: pgbouncer-certs
          projected:
            sources:
              ## CLIENT TLS FILES (CHART GENERATED)
              - secret:
                  name: XXX-pgbouncer-certs
                  items:
                    - key: client.key
                      path: client.key
                    - key: client.crt
                      path: client.crt

However with 8.7.1 , projected.sources is empty :

        - name: pgbouncer-certs
          projected:
            sources:

Seems like issue comes from https://github.com/airflow-helm/charts/pull/718 .

See below section for custom values I've used under ``pgbouncer``` section . Thanks.

Relevant Logs

│ Error: error validating "": error validating data: ValidationError(Deployment.spec.template.spec.volumes[1].projected): missing required field "sources" in io.k8s.api.core.v1.ProjectedVolumeSource

Custom Helm Values

pgbouncer:
  enabled: true
  image:
    repository: XXXX/pgbouncer
    tag: 1.18.0-patch.1
    pullPolicy: IfNotPresent
  maxClientConnections: 400
  poolSize: 25
thesuperzapper commented 11 months ago

@enrique-ayala thanks for reporting, just want to confirm if this is preventing you from installing, or is just throwing a warning?

enrique-ayala commented 11 months ago

Yes, this is preventing us to install it. Thanks for checking.

thesuperzapper commented 11 months ago

@enrique-ayala While I agree that there is a bug (we need to change the if statement to prevent the empty project volume), I am trying to understand what makes your situation not allow installing, when many others have had no problem.

What are the exact commands you are running when you get the error?

enrique-ayala commented 11 months ago

I'm using terraform from a build/release pipeline . Command executed looks like : terraform apply -var-file="varfile.tfvars"

thesuperzapper commented 11 months ago

@enrique-ayala what is the terraform code you are referencing the chart with?

enrique-ayala commented 11 months ago

This is via Azure Pipeline :

          - task: TerraformCLI@0
            displayName: 'terraform apply'
            env: ${{ parameters.env }}
            inputs:
              command: apply
              commandOptions: '-var-file=env_values/$(terraform_workspace_environment).tfvars  -var kube_config_path=$(KUBECONFIG)'
              workingDirectory: $(terraform_working_directory)

Where terraformcli refers to extension: https://marketplace.visualstudio.com/items?itemName=ms-devlabs.custom-terraform-tasks&ssr=false#overview

and terraform version: terraformVersion: 1.3.4

thesuperzapper commented 11 months ago

@enrique-ayala I understand that part, I was meaning what helm provider are you using?

Presumably, you are using the helm one: https://registry.terraform.io/providers/hashicorp/helm/latest/docs

Either way, can you share the specific terraform code?

enrique-ayala commented 11 months ago

Oh got it , helm provider version is v2.7.1:

- Installing hashicorp/helm v2.7.1...
- Installed hashicorp/helm v2.7.1 (signed by HashiCorp)

Which seems to refer to helm 3.9.4 (https://github.com/hashicorp/terraform-provider-helm/releases/tag/v2.7.0)

thesuperzapper commented 11 months ago

@enrique-ayala I am actually wanting your terraform code, lol.

For example, you might be using the helm_release resource, or you could be using some other method.

The following is an example of a helm_release pattern, but can you share what you are actually using?

resource "helm_release" "example" {
  name       = "my-redis-release"
  repository = "https://charts.bitnami.com/bitnami"
  chart      = "redis"
  version    = "6.0.1"

  values = [
    "${file("values.yaml")}"
  ]

  set {
    name  = "cluster.enabled"
    value = "true"
  }

  set {
    name  = "metrics.enabled"
    value = "true"
  }

  set {
    name  = "service.annotations.prometheus\\.io/port"
    value = "9127"
    type  = "string"
  }
}
enrique-ayala commented 11 months ago

Let me try to put as much as possible ( we have a strict policy regarding exposing internal configs) :

# main.tf
resource "helm_release" "airflow" {
    name       = var.release_name
    repository = "https://airflow-helm.github.io/charts"
    chart      = var.chart_name
    version    = var.chart_version
    namespace  = var.namespace
    values = [
        templatefile("chart_values/airflow.yaml", {PVC_XXX = var.xxx_pvc_name}),
        file("chart_values/airflow-${var.environment_name}.yaml")
    ]
}
# vars.tf
variable "xxx_pvc_name" {
  type      = string
  nullable  = false
  description = "PVC name to use as working volume (/mnt/work)"
}  

variable "release_name" {
  type      = string
  nullable  = false
  default = "airflow"
  description = "Helm release name for airflow"
}  

variable "chart_name" {
  type      = string
  nullable  = false
  default = "airflow"
  description = "Helm chart name for airflow"
}

variable "chart_version" {
  type      = string
  nullable  = false
  default = "airflow"
  description = "Helm chart version for airflow"
}

variable "kube_config_path" {
   type     = string
   nullable = false
   description = "Kubernetes config yaml file"
 }

variable "namespace" {
   type     = string
   nullable = false
   description = "Kubernetes namespace name to deploy chart"
}

variable "environment_name" {
   type     = string
   nullable = false
}
terraform {
backend "azurerm" {
# XXXX
  }
}
provider "azurerm" {
# XXX
}

provider "helm" {
  kubernetes {
    config_path = var.kube_config_path
  }

}

terraform {
  required_version = ">=1.1.9"
}

terraform {
  required_providers {
    azurerm = {
      source  = "hashicorp/azurerm"
      version = "3.14.0"
    }
    helm = {
      version = "2.7.1"
    }
  }
}

And a sample from a tfvars file :

namespace                           = "airflow"
chart_name                          = "airflow"
chart_version                       = "8.7.1"

Thanks again for checking this.

thesuperzapper commented 11 months ago

@enrique-ayala as a temporary fix (until a chart version that does not have the invalid resource schema is released), you can disable Terraform's built-in validation setting the disable_openapi_validation config to true on the helm_release resource.

enrique-ayala commented 11 months ago

thanks @thesuperzapper ! I'll test it .

thesuperzapper commented 10 months ago

@enrique-ayala I have released chart version 8.8.0 which contains a fix for this schema error.

enrique-ayala commented 10 months ago

Thanks @thesuperzapper ! Worked with no issue.