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

Add NodePort Option to the values schema #26812

Closed
2 tasks done
joseph-max-coalfire opened this issue Sep 30, 2022 · 2 comments · Fixed by #26945
Closed
2 tasks done

Add NodePort Option to the values schema #26812

joseph-max-coalfire opened this issue Sep 30, 2022 · 2 comments · Fixed by #26945
Assignees
Labels
area:helm-chart Airflow Helm Chart kind:bug This is a clearly a bug

Comments

@joseph-max-coalfire
Copy link
Contributor

joseph-max-coalfire commented Sep 30, 2022

Official Helm Chart version

1.6.0 (latest released)

Apache Airflow version

2.3.4

Kubernetes Version

1.23

Helm Chart configuration

# shortened values.yaml file  
webserver:
    service:
      type: NodePort
      ports:
        - name: airflow-ui
          port: 80
          targetPort: airflow-ui
          nodePort: 8081  # Note this line does not work, this is what'd be nice to have for defining nodePort

Docker Image customisations

No response

What happened

Supplying nodePort like in the above Helm Chart Configuration example fails with an error saying a value of nodePort is not supported.

What you think should happen instead

It'd be nice if we could define the nodePort we want the airflow-webserver service to listen on at launch. As it currently stands, supplying nodePort like in the above values.yaml example will fail, saying nodePort cannot be supplied. The workaround is to manually edit the webserver service post-deployment and specify the desired port for nodePort.

I looked at the way the webserver service template file is set up, and the logic there should allow this, but I believe the missing definition in the schema.json file is causing this to error out.

How to reproduce

Attempt to install the Airflow Helm chart using the above values.yaml config

Anything else

No response

Are you willing to submit PR?

  • Yes I am willing to submit a PR!

Code of Conduct

@joseph-max-coalfire joseph-max-coalfire added area:helm-chart Airflow Helm Chart kind:bug This is a clearly a bug labels Sep 30, 2022
@boring-cyborg
Copy link

boring-cyborg bot commented Sep 30, 2022

Thanks for opening your first issue here! Be sure to follow the issue template!

@mik-laj
Copy link
Member

mik-laj commented Oct 1, 2022

Feel free.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:helm-chart Airflow Helm Chart kind:bug This is a clearly a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants