Merge pull request #30140 from MaterializeInc/doy-cleanup

a few more cleanups to orchestratord and the helm charts

misc/helm-charts/environmentd/README.md

@@ -52,7 +52,7 @@ The following table lists the configurable parameters of the Materialize environ
 | `environment.environmentdMemoryAllocation` |  | ``"1Gi"`` |
 | `environment.forceRollout` |  | ``"33333333-3333-3333-3333-333333333333"`` |
 | `environment.inPlaceRollout` |  | ``false`` |
-| `environment.name` |  | ``"environment-12345678-1234-1234-1234-123456789012"`` |
+| `environment.name` |  | ``"12345678-1234-1234-1234-123456789012"`` |
 | `environment.requestRollout` |  | ``"22222222-2222-2222-2222-222222222222"`` |
 | `environment.secret.metadataBackendUrl` |  | ``"postgres://materialize_user:materialize_pass@postgres.materialize-environment.svc.cluster.local:5432/materialize_db?sslmode=disable"`` |
 | `environment.secret.persistBackendUrl` |  | ``"s3://minio:minio123@bucket/12345678-1234-1234-1234-123456789012?endpoint=http%3A%2F%2Fminio.materialize-environment.svc.cluster.local%3A9000&region=minio"`` |

misc/helm-charts/environmentd/values.yaml

@@ -17,7 +17,7 @@ namespace:
 # Environment configuration for Materialize
 environment:
   # The name of the environment to be deployed, this should be a UUID
-  name: "environment-12345678-1234-1234-1234-123456789012"
+  name: "12345678-1234-1234-1234-123456789012"
   # Docker image reference for the Materialize `environmentd` service
   environmentdImageRef: "materialize/environmentd:v0.122.0-dev.0--pr.g8bb641fc00c77f98ba5556dcdca43670776eacfa"
   # Optional additional arguments to be passed to `environmentd`

misc/helm-charts/operator/README.md

@@ -37,20 +37,22 @@ The following table lists the configurable parameters of the Materialize operato
 
 | Parameter | Description | Default |
 |-----------|-------------|---------|
+| `clusterd.nodeSelector` |  | ``{}`` |
+| `environmentd.nodeSelector` |  | ``{}`` |
 | `namespace.create` |  | ``false`` |
 | `namespace.name` |  | ``"materialize"`` |
 | `networkPolicies.enabled` |  | ``false`` |
 | `networkPolicies.useNativeKubernetesPolicy` |  | ``true`` |
 | `observability.enabled` |  | ``false`` |
 | `observability.prometheus.enabled` |  | ``false`` |
 | `operator.args.cloudProvider` |  | ``"local"`` |
-| `operator.args.environmentdTargetArch` |  | ``"arm64"`` |
 | `operator.args.localDevelopment` |  | ``true`` |
 | `operator.args.region` |  | ``"kind"`` |
 | `operator.args.startupLogFilter` |  | ``"INFO,mz_orchestratord=TRACE"`` |
 | `operator.image.pullPolicy` |  | ``"IfNotPresent"`` |
 | `operator.image.repository` |  | ``"materialize/orchestratord"`` |
 | `operator.image.tag` |  | ``"v0.122.0-dev.0--pr.g8bb641fc00c77f98ba5556dcdca43670776eacfa"`` |
+| `operator.nodeSelector` |  | ``{}`` |
 | `operator.resources.limits.cpu` |  | ``"500m"`` |
 | `operator.resources.limits.memory` |  | ``"512Mi"`` |
 | `operator.resources.requests.cpu` |  | ``"100m"`` |

misc/helm-charts/operator/templates/deployment.yaml

@@ -26,6 +26,10 @@ spec:
         {{- include "materialize-operator.selectorLabels" . | nindent 8 }}
     spec:
       serviceAccountName: {{ include "materialize-operator.serviceAccountName" . }}
+      {{- if .Values.operator.nodeSelector }}
+      nodeSelector:
+        {{- toYaml .Values.operator.nodeSelector | nindent 8 }}
+      {{- end }}
       containers:
       - name: {{ .Chart.Name }}
         image: "{{ .Values.operator.image.repository }}:{{ .Values.operator.image.tag }}"
@@ -37,6 +41,12 @@ spec:
         {{- if .Values.operator.args.localDevelopment }}
         - "--local-development"
         {{- end }}
-        - "--environmentd-target-arch={{ .Values.operator.args.environmentdTargetArch }}"
+        - "--image-pull-policy={{ kebabcase .Values.operator.image.pullPolicy }}"
+        {{- range $key, $value := .Values.environmentd.nodeSelector }}
+        - "--environmentd-node-selector={{ $key }}={{ $value }}"
+        {{- end }}
+        {{- range $key, $value := .Values.clusterd.nodeSelector }}
+        - "--clusterd-node-selector={{ $key }}={{ $value }}"
+        {{- end }}
         resources:
           {{- toYaml .Values.operator.resources | nindent 10 }}

misc/helm-charts/operator/templates/serviceaccount.yaml

@@ -12,6 +12,7 @@ apiVersion: v1
 kind: ServiceAccount
 metadata:
   name: {{ include "materialize-operator.serviceAccountName" . }}
+  namespace: {{ .Values.namespace.name }}
   labels:
     {{- include "materialize-operator.labels" . | nindent 4 }}
 

misc/helm-charts/operator/values.yaml

@@ -25,8 +25,8 @@ operator:
     region: "kind"
     # Flag to indicate whether the environment is for local development
     localDevelopment: true
-    # Target architecture for environmentd (set to "arm64" for ARM systems, can also be "amd64" for Intel/AMD)
-    environmentdTargetArch: "arm64" # amd64
+  # Node selector to use for the operator pod
+  nodeSelector: {}
   resources:
     # Resources requested by the operator for CPU and memory
     requests:
@@ -36,6 +36,14 @@ operator:
     limits:
       memory: 512Mi
 
+environmentd:
+  # Node selector to use for environmentd pods spawned by the operator
+  nodeSelector: {}
+
+clusterd:
+  # Node selector to use for clusterd pods spawned by the operator
+  nodeSelector: {}
+
 # RBAC (Role-Based Access Control) settings
 rbac:
   # Whether to create necessary RBAC roles and bindings

src/cloud-resources/src/crd/materialize.rs

@@ -82,26 +82,6 @@ pub mod v1alpha1 {
             format!("materialize-backend-{}", self.name_unchecked())
         }
 
-        pub fn cockroach_database_name(&self) -> String {
-            format!("materialize_{}", self.name_unchecked())
-        }
-
-        pub fn cockroach_role_name(&self) -> String {
-            self.cockroach_database_name()
-        }
-
-        pub fn oidc_sub(&self) -> String {
-            format!(
-                "system:serviceaccount:{}:{}",
-                self.name_unchecked(),
-                self.service_account_name()
-            )
-        }
-
-        pub fn persistence_bucket_prefix(&self) -> String {
-            self.oidc_sub()
-        }
-
         pub fn namespace(&self) -> String {
             self.meta().namespace.clone().unwrap()
         }
@@ -118,28 +98,20 @@ pub mod v1alpha1 {
             self.name_unchecked()
         }
 
-        pub fn environmentd_statefulset_name(generation: u64) -> String {
-            format!("environmentd-{generation}")
-        }
-
-        pub fn environmentd_container_name() -> String {
-            "environmentd".to_string()
-        }
-
-        pub fn environmentd_service_name() -> String {
-            "environmentd".to_string()
+        pub fn environmentd_statefulset_name(&self, generation: u64) -> String {
+            format!("environmentd-{}-{generation}", self.name_unchecked())
         }
 
-        pub fn environmentd_generation_service_name(generation: u64) -> String {
-            format!("environmentd-{generation}")
+        pub fn environmentd_service_name(&self) -> String {
+            format!("environmentd-{}", self.name_unchecked())
         }
 
-        pub fn environmentd_pod_name(generation: u64) -> String {
-            format!("{}-0", Self::environmentd_statefulset_name(generation))
+        pub fn environmentd_generation_service_name(&self, generation: u64) -> String {
+            format!("environmentd-{}-{generation}", self.name_unchecked())
         }
 
-        pub fn persist_pubsub_service_name(generation: u64) -> String {
-            format!("persist-pubsub-{generation}")
+        pub fn persist_pubsub_service_name(&self, generation: u64) -> String {
+            format!("persist-pubsub-{}-{generation}", self.name_unchecked())
         }
 
         pub fn default_labels(&self) -> BTreeMap<String, String> {

src/orchestrator-kubernetes/src/lib.rs

@@ -116,6 +116,16 @@ impl fmt::Display for KubernetesImagePullPolicy {
     }
 }
 
+impl KubernetesImagePullPolicy {
+    pub fn as_kebab_case_str(&self) -> &'static str {
+        match self {
+            Self::Always => "always",
+            Self::IfNotPresent => "if-not-present",
+            Self::Never => "never",
+        }
+    }
+}
+
 /// An orchestrator backed by Kubernetes.
 pub struct KubernetesOrchestrator {
     client: Client,

src/orchestratord/src/controller/materialize.rs

@@ -21,9 +21,9 @@ use tracing::{debug, trace};
 
 use crate::metrics::Metrics;
 use mz_cloud_resources::crd::materialize::v1alpha1::{Materialize, MaterializeStatus};
+use mz_orchestrator_kubernetes::KubernetesImagePullPolicy;
 use mz_orchestrator_tracing::TracingCliArgs;
-use mz_ore::cast::CastFrom;
-use mz_ore::instrument;
+use mz_ore::{cast::CastFrom, cli::KeyValueArg, instrument};
 
 mod resources;
 
@@ -35,26 +35,21 @@ pub struct Args {
     region: String,
     #[clap(long)]
     local_development: bool,
-    #[clap(long)]
-    environmentd_target_arch: String,
 
     #[clap(flatten)]
     aws_info: AwsInfo,
 
     #[clap(long)]
-    persist_bucket: Option<String>,
-
-    #[clap(long)]
-    frontegg_jwk: Option<String>,
-    #[clap(long)]
-    frontegg_url: Option<String>,
+    scheduler_name: Option<String>,
     #[clap(long)]
-    frontegg_admin_role: Option<String>,
+    enable_security_context: bool,
 
     #[clap(long)]
-    scheduler_name: Option<String>,
+    environmentd_node_selector: Vec<KeyValueArg<String, String>>,
     #[clap(long)]
-    enable_security_context: bool,
+    clusterd_node_selector: Vec<KeyValueArg<String, String>>,
+    #[clap(long, default_value = "always", arg_enum)]
+    image_pull_policy: KubernetesImagePullPolicy,
 
     #[clap(long, default_value_t = default_cluster_replica_sizes())]
     environmentd_cluster_replica_sizes: String,
@@ -348,7 +343,7 @@ impl k8s_controller::Context for Context {
                         resources.promote_services(&client, &mz.namespace()).await?;
                         if increment_generation {
                             resources
-                                .teardown_generation(&client, &mz.namespace(), active_generation)
+                                .teardown_generation(&client, mz, active_generation)
                                 .await?;
                         }
                         self.update_status(
@@ -376,7 +371,7 @@ impl k8s_controller::Context for Context {
                     }
                     Err(e) => {
                         resources
-                            .teardown_generation(&client, &mz.namespace(), next_generation)
+                            .teardown_generation(&client, mz, next_generation)
                             .await?;
                         self.update_status(
                             &mz_api,
@@ -410,7 +405,7 @@ impl k8s_controller::Context for Context {
                 let mut needs_update = mz.conditions_need_update();
                 if mz.update_in_progress() {
                     resources
-                        .teardown_generation(&client, &mz.namespace(), next_generation)
+                        .teardown_generation(&client, mz, next_generation)
                         .await?;
                     needs_update = true;
                 }
@@ -448,7 +443,7 @@ impl k8s_controller::Context for Context {
             let mut needs_update = mz.conditions_need_update() || mz.rollout_requested();
             if mz.update_in_progress() {
                 resources
-                    .teardown_generation(&client, &mz.namespace(), next_generation)
+                    .teardown_generation(&client, mz, next_generation)
                     .await?;
                 needs_update = true;
             }