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

Allow to define SpreadTopologyContraints in values for server and worker #399

Closed

Conversation

GuilleAmutio
Copy link

Hi,

This PR allows user to define the SpreadTopologyConstraints for the charts of server and worker.

This feature was requested in #360

@GuilleAmutio
Copy link
Author

Just to add, worker lint is failing and dont know exactly why.

I removed what i did on the worker, and still failed the lint. Could someone recheck? Thanks

@mitchnielsen
Copy link
Contributor

Current error:

==> Linting charts/prefect-worker
[INFO] Chart.yaml: icon is recommended
Error:  values.yaml: Expected: valid schema, given: Invalid JSON
Error:  templates/: values don't meet the specifications of the schema(s) in the following chart(s):
prefect-worker:
Expected: valid schema, given: Invalid JSON

@mitchnielsen
Copy link
Contributor

@GuilleAmutio you'll need two fixes here:

First, the indentation of the labels needs to be corrected:

diff --git a/charts/prefect-worker/templates/deployment.yaml b/charts/prefect-worker/templates/deployment.yaml
index 3cf8a3f..d92cd49 100644
--- a/charts/prefect-worker/templates/deployment.yaml
+++ b/charts/prefect-worker/templates/deployment.yaml
@@ -229,7 +229,7 @@ spec:
               whenUnsatisfiable: {{ .whenUnsatisfiable }}
               labelSelector:
                 matchLabels:
-                  {{- toYaml .labelSelector.matchLabels | nindent 12 }}
+                  {{- toYaml .labelSelector.matchLabels | nindent 18 }}
             {{- end }}
           {{- end }}
           volumeMounts:

Second, you'll need to fix the schema:

diff --git a/charts/prefect-worker/values.schema.json b/charts/prefect-worker/values.schema.json
index e3a2a04..1149b2d 100644
--- a/charts/prefect-worker/values.schema.json
+++ b/charts/prefect-worker/values.schema.json
@@ -492,11 +492,11 @@
                       }
                     }
                   }
-                }
+                },
+                "required": ["maxSkew", "topologyKey", "whenUnsatisfiable", "labelSelector"]
               },
-              "required": ["maxSkew", "topologyKey", "whenUnsatisfiable", "labelSelector"]
-            },
-            "description": "Array of constraints for topology spread of the pods."
+              "description": "Array of constraints for topology spread of the pods."
+            }
           }
         },
         "livenessProbe": {

@GuilleAmutio
Copy link
Author

Hi @mitchnielsen sorry for the delay, fixed, thank you very much for the comment before!

@GuilleAmutio
Copy link
Author

Closing this MR due to conflicts with email used. Gonna reopen

@GuilleAmutio GuilleAmutio deleted the feature/allow_spread_topology branch October 22, 2024 06:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants