breaking(prefect-server): remove confusing & duplicative environment variables #450

jamiezieziula · 2025-02-14T17:21:31Z

This PR contains a breaking change. See the upgrading document for details on how to upgrade. Specifically, this PR removes the prefectApiUrl and prefectApiHost values. Instead, the chart will now use the prefectUiApiUrl value to define the URL that is provided to the UI by and for the API

Testing

Authenticate to any Kubernetes cluster
Clone the repo locally, checkout this branch and change directory to the prefect-server chart

git clone git@github.com:PrefectHQ/prefect-helm.git
git checkout adjust-prefect-server-settings
cd prefect-helm/charts/prefect-server

Install the helm chart without adjusting anything on the values.yaml file

helm install prefect-server .

You should observe the prefect-server and postgresql pods come up healthy. You can port forward the server service and interact with the UI
Modify the values.yaml file to enable a separate deployment for the background services

backgroundServices:
  runAsSeparateDeployment: true

Upgrade the helm chart

helm upgrade prefect-server .

You should see all three pods (now including a dedicated one for the background services) come up healthy.

Resolves https://linear.app/prefect/issue/PLA-1051/fix-prefect-helm-server-values-and-docs-re-server-env-vars

…prefect-server-settings

jamiezieziula · 2025-02-20T22:11:00Z

charts/prefect-server/values.yaml

-    prefectUiApiUrl: ""
-    # -- sets PREFECT_UI_URL
-    prefectUiUrl: ""
+    # -- sets PREFECT_UI_API_URL; If you want to connect to the UI form somewhere external to the cluster (like via an ingress), you need to set this value to the ingress URL (e.g. http://app.internal.prefect.com/api).


@zzstoatzz @desertaxle - let me know if my description makes sense here

Makes sense to me!

jamiezieziula · 2025-02-20T22:11:32Z

charts/prefect-server/values.yaml

-    # -- sets PREFECT_UI_API_URL
-    prefectUiApiUrl: ""
-    # -- sets PREFECT_UI_URL
-    prefectUiUrl: ""


this was used solely to display the connection url in the notes.txt. I've updated this to infer the UI url from the prefectUiApiUrl

jamiezieziula · 2025-02-20T22:12:28Z

charts/prefect-server/values.yaml

-    prefectApiUrl: http://localhost:4200/api
-
-    # -- sets PREFECT_SERVER_API_HOST
-    prefectApiHost: 0.0.0.0


these are both redundant because:

PREFECT_SERVER_API_HOST it is only used to construct the PREFECT_UI_API_URL if not provided

PREFECT_API_URL is primarily used by the client, and is only used server side if PREFECT_UI_API_URL is not set

With these removed, maybe we can add a template to _validation.tpl to return an error/warning if the user tries to configure these removed values.

jamiezieziula · 2025-02-20T22:12:50Z

charts/prefect-server/values.schema.json

-        "prefectApiUrl": {
-          "type": "string",
-          "title": "Prefect API URL",
-          "description": "sets PREFECT_API_URL"
-        },
-        "prefectApiHost": {
-          "type": "string",
-          "title": "Prefect API Host",
-          "description": "sets PREFECT_SERVER_API_HOST"
-        },


these we're left over and should have been removed in a previous PR

jamiezieziula · 2025-02-20T22:13:48Z

charts/prefect-server/templates/server-deployment.yaml

            - name: PREFECT_SERVER_API_PORT
              value: {{ .Values.service.targetPort | quote }}
            - name: PREFECT_UI_ENABLED
              value: {{ .Values.server.uiConfig.enabled | quote }}
-            {{- if .Values.server.uiConfig.prefectUiApiUrl }}


remove if/then configuration as this is now a required setting

jamiezieziula · 2025-02-20T22:14:04Z

charts/prefect-server/templates/background-services-deployment.yaml

@@ -71,14 +71,10 @@ spec:
          env:
            - name: HOME
              value: /home/prefect
-            - name: PREFECT_API_URL


we never needed to set these env vars on the background services depoyment

desertaxle

LGTM!

charts/prefect-server/values.yaml

jimid27

lgtm

jamiezieziula · 2025-02-24T19:16:50Z

charts/prefect-server/templates/_validation.tpl

-  {{- if or (lt $major 3) (and (eq $major 3) (lt $minor 1)) (and (eq $major 3) (eq $minor 1) (lt $patch 13)) -}}
-    {{- fail "When running background services separately, Prefect version must be 3.1.13 or higher" -}}
-  {{- end -}}
+{{- define "prefect-server.validatePrefectServerApiSettings" -}}


if any of these three values are set, the validation will fail the install/upgrade and alert the user on what needs to be changed

jamiezieziula · 2025-02-24T19:17:01Z

charts/prefect-server/templates/background-services-deployment.yaml

@@ -1,4 +1,5 @@
 {{- if .Values.backgroundServices.runAsSeparateDeployment }}
+{{- include "prefect-server.validatePrefectVersion" . }}


this calls the validation helper

Can we just call the validation helper in NOTES.txt? It's been a little while since I've used this, but I think that's where I've called it in the past so it's in a central location instead of being tied to a specific template.

jamiezieziula · 2025-02-24T19:17:16Z

charts/prefect-server/templates/server-deployment.yaml

@@ -1,3 +1,4 @@
+{{- include "prefect-server.validatePrefectServerApiSettings" . }}


this calls the validation helper

jamiezieziula · 2025-02-24T19:17:42Z

charts/prefect-server/values.schema.json

@@ -42,7 +42,7 @@
          "type": "object",
          "title": "Prefect",
          "description": "global prefect configuration",
-          "additionalProperties": false,
+          "additionalProperties": true,


this allows for the validation helper to be the one to fail the helm install/upgrade, rather than the json schema. It will provide more direct guidance.

charts/prefect-server/templates/_validation.tpl

mitchnielsen · 2025-02-24T19:59:13Z

charts/prefect-server/templates/background-services-deployment.yaml

@@ -1,4 +1,5 @@
 {{- if .Values.backgroundServices.runAsSeparateDeployment }}
+{{- include "prefect-server.validatePrefectVersion" . }}


Can we just call the validation helper in NOTES.txt? It's been a little while since I've used this, but I think that's where I've called it in the past so it's in a central location instead of being tied to a specific template.

Co-authored-by: Mitchell Nielsen <mitchnielsen@users.noreply.github.com>

charts/prefect-server/templates/_validation.tpl

jamie zieziula added 2 commits February 14, 2025 11:18

remove references to PREFECT_API_URL and PREFECT_SERVER_API_HOST

460d1e8

add default value for ui api url

02b22dd

jamiezieziula added the breaking label Feb 14, 2025

jamie zieziula added 2 commits February 20, 2025 16:55

Merge branch 'main' of github.com:PrefectHQ/prefect-helm into adjust-…

8c1f064

…prefect-server-settings

add comment, remove if/then logic on UI value

490ffbe

jamiezieziula commented Feb 20, 2025

View reviewed changes

initial upgrade guide

08f828c

jamiezieziula changed the title ~~chore: remove unnecessary environment variables and clear up documentation~~ chore: remove confusing & duplicative environment variables Feb 20, 2025

jamie zieziula added 2 commits February 20, 2025 17:27

typo

2d44115

rmeove 'like'

2d6cff5

jamiezieziula marked this pull request as ready for review February 21, 2025 17:07

jamiezieziula requested a review from a team as a code owner February 21, 2025 17:07

desertaxle approved these changes Feb 21, 2025

View reviewed changes

mitchnielsen reviewed Feb 21, 2025

View reviewed changes

charts/prefect-server/values.yaml Outdated Show resolved Hide resolved

add docs

1a0707e

jamiezieziula requested a review from mitchnielsen February 21, 2025 19:08

This comment was marked as duplicate.

Sign in to view

jimid27 approved these changes Feb 21, 2025

View reviewed changes

add validation for breaking changes, fix prefect version validation

1c0a503

jamiezieziula commented Feb 24, 2025

View reviewed changes

jamie zieziula added 2 commits February 24, 2025 14:18

test

6a5b213

fix readme

4c5e434

jamie zieziula added 2 commits February 24, 2025 14:23

now that validation works, make sure tests pass validation

a966b5e

wrong indent

1b0a811

mitchnielsen reviewed Feb 24, 2025

View reviewed changes

jamie zieziula and others added 7 commits February 24, 2025 15:02

correct tag name

f8128c3

move validation to notes.txt

786a638

remove newline

bbc5e30

remove duplicate

47a93ba

Update charts/prefect-server/templates/_validation.tpl

70a67c8

Co-authored-by: Mitchell Nielsen <mitchnielsen@users.noreply.github.com>

Update charts/prefect-server/templates/_validation.tpl

48a1785

Co-authored-by: Mitchell Nielsen <mitchnielsen@users.noreply.github.com>

Update charts/prefect-server/templates/_validation.tpl

0501d3d

Co-authored-by: Mitchell Nielsen <mitchnielsen@users.noreply.github.com>

jamiezieziula requested a review from mitchnielsen February 24, 2025 20:17

jamiezieziula commented Feb 24, 2025

View reviewed changes

charts/prefect-server/templates/_validation.tpl Outdated Show resolved Hide resolved

jamiezieziula commented Feb 24, 2025

View reviewed changes

charts/prefect-server/templates/_validation.tpl Show resolved Hide resolved

jamiezieziula and others added 3 commits February 24, 2025 15:18

Update charts/prefect-server/templates/_validation.tpl

c6a5a69

Update charts/prefect-server/templates/_validation.tpl

25be1e7

missing end

2659895

jamiezieziula changed the title ~~chore: remove confusing & duplicative environment variables~~ breaking change: remove confusing & duplicative environment variables Feb 24, 2025

Merge branch 'main' into adjust-prefect-server-settings

1858b80

jamiezieziula removed the request for review from mitchnielsen February 25, 2025 01:41

jamiezieziula merged commit 00fb799 into main Feb 25, 2025
13 checks passed

jamiezieziula deleted the adjust-prefect-server-settings branch February 25, 2025 01:42

jamiezieziula mentioned this pull request Feb 25, 2025

breaking(prefect-server): update basicAuth settings to be nested under the right selfHostedServerApiConfig key #456

Merged

8 tasks

jamiezieziula changed the title ~~breaking change: remove confusing & duplicative environment variables~~ breaking(prefect-server): remove confusing & duplicative environment variables Feb 27, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

breaking(prefect-server): remove confusing & duplicative environment variables #450

breaking(prefect-server): remove confusing & duplicative environment variables #450

jamiezieziula commented Feb 14, 2025 •

edited

Loading

jamiezieziula Feb 20, 2025

desertaxle Feb 20, 2025

jamiezieziula Feb 20, 2025

jamiezieziula Feb 20, 2025 •

edited

Loading

mitchnielsen Feb 21, 2025

jamiezieziula Feb 20, 2025

jamiezieziula Feb 20, 2025

jamiezieziula Feb 20, 2025

desertaxle left a comment

This comment was marked as duplicate.

jimid27 left a comment

jamiezieziula Feb 24, 2025

jamiezieziula Feb 24, 2025

mitchnielsen Feb 24, 2025

jamiezieziula Feb 24, 2025

jamiezieziula Feb 24, 2025

jamiezieziula Feb 24, 2025

mitchnielsen Feb 24, 2025

		@@ -1,4 +1,5 @@
		{{- if .Values.backgroundServices.runAsSeparateDeployment }}
		{{- include "prefect-server.validatePrefectVersion" . }}

		@@ -1,3 +1,4 @@
		{{- include "prefect-server.validatePrefectServerApiSettings" . }}

breaking(prefect-server): remove confusing & duplicative environment variables #450

breaking(prefect-server): remove confusing & duplicative environment variables #450

Conversation

jamiezieziula commented Feb 14, 2025 • edited Loading

Testing

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jamiezieziula Feb 20, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

desertaxle left a comment

Choose a reason for hiding this comment

This comment was marked as duplicate.

jimid27 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jamiezieziula commented Feb 14, 2025 •

edited

Loading

jamiezieziula Feb 20, 2025 •

edited

Loading