From bbd58a20b08749ae3a9f097f97b8c850d9cdbede Mon Sep 17 00:00:00 2001 From: Fabien Boucher Date: Thu, 19 Oct 2023 08:31:55 +0000 Subject: [PATCH] nodepool-builder - log expose - slight refactor This change slightly refators the build logs expose: - No longer use /var/log/nodepool and store logs in the /var/lib/nodepool/builds directory. We can then remove an EmptyDir. - No longer need for the Route rewrite annotation Also this changes enable a way to update route Annotations. Change-Id: I2a88588260ddafd49067997bcdd5d21a5ad4d704 --- controllers/nodepool.go | 24 +++++++---------- .../static/nodepool/generate-config.sh | 1 + .../static/nodepool/httpd-build-logs-dir.conf | 2 +- controllers/utils.go | 26 +++++++++++++++---- .../check-service-uri/tasks/main.yaml | 4 +-- .../tasks/main.yaml | 2 +- roles/post/get-k8s-resources/tasks/main.yaml | 1 + .../get-nodepool-builds-logs/tasks/main.yaml | 4 +-- 8 files changed, 38 insertions(+), 26 deletions(-) diff --git a/controllers/nodepool.go b/controllers/nodepool.go index 53f861ab..dcada6cc 100644 --- a/controllers/nodepool.go +++ b/controllers/nodepool.go @@ -350,7 +350,6 @@ func (r *SFController) DeployNodepoolBuilder(statsdExporterVolume apiv1.Volume, base.MkVolumeSecret(NodepoolProvidersSecretsName), base.MkEmptyDirVolume("nodepool-config"), base.MkEmptyDirVolume("nodepool-home-ssh"), - base.MkEmptyDirVolume("nodepool-log"), r.commonToolingVolume(), { Name: "nodepool-builder-ssh-key", @@ -380,10 +379,6 @@ func (r *SFController) DeployNodepoolBuilder(statsdExporterVolume apiv1.Volume, Name: builderIdent, MountPath: "/var/lib/nodepool", }, - { - Name: "nodepool-log", - MountPath: "/var/log/nodepool", - }, configScriptVolumeMount, { Name: "nodepool-tooling-vol", @@ -426,12 +421,12 @@ func (r *SFController) DeployNodepoolBuilder(statsdExporterVolume apiv1.Volume, "statsd_mapping": utils.Checksum([]byte(nodepoolStatsdMappingConfig)), // When the Secret ResourceVersion field change (when edited) we force a nodepool-builder restart "nodepool-providers-secrets": string(nodepoolProvidersSecrets.ResourceVersion), - "serial": "7", + "serial": "8", } initContainer := base.MkContainer("nodepool-builder-init", base.BusyboxImage) - initContainer.Command = []string{"bash", "-c", "mkdir -p ~/dib; /usr/local/bin/generate-config.sh"} + initContainer.Command = []string{"bash", "-c", "mkdir -p ~/dib ~/builds; /usr/local/bin/generate-config.sh"} initContainer.Env = append(r.getNodepoolConfigEnvs(), base.MkEnvVar("NODEPOOL_CONFIG_FILE", "nodepool-builder.yaml"), ) @@ -469,8 +464,9 @@ func (r *SFController) DeployNodepoolBuilder(statsdExporterVolume apiv1.Volume, buildLogsContainer := base.MkContainer("build-logs-httpd", HTTPDImage) buildLogsContainer.VolumeMounts = []apiv1.VolumeMount{ { - Name: "nodepool-log", - MountPath: "/var/www/html/dib", + Name: builderIdent, + SubPath: "builds", + MountPath: "/var/www/html/builds", }, { Name: "nodepool-builder-extra-config-vol", @@ -481,9 +477,9 @@ func (r *SFController) DeployNodepoolBuilder(statsdExporterVolume apiv1.Volume, buildLogsContainer.Ports = []apiv1.ContainerPort{ base.MkContainerPort(buildLogsHttpdPort, buildLogsHttpdPortName), } - buildLogsContainer.ReadinessProbe = base.MkReadinessHTTPProbe("/dib", buildLogsHttpdPort) - buildLogsContainer.StartupProbe = base.MkStartupHTTPProbe("/dib", buildLogsHttpdPort) - buildLogsContainer.LivenessProbe = base.MkLiveHTTPProbe("/dib", buildLogsHttpdPort) + buildLogsContainer.ReadinessProbe = base.MkReadinessHTTPProbe("/builds", buildLogsHttpdPort) + buildLogsContainer.StartupProbe = base.MkStartupHTTPProbe("/builds", buildLogsHttpdPort) + buildLogsContainer.LivenessProbe = base.MkLiveHTTPProbe("/builds", buildLogsHttpdPort) nb.Spec.Template.Spec.Containers = append(nb.Spec.Template.Spec.Containers, buildLogsContainer, ) @@ -508,9 +504,7 @@ func (r *SFController) DeployNodepoolBuilder(statsdExporterVolume apiv1.Volume, pvcReadiness := r.reconcileExpandPVC(builderIdent+"-"+builderIdent+"-0", r.cr.Spec.Nodepool.Builder.Storage) routeReady := r.ensureHTTPSRoute(r.cr.Name+"-nodepool-builder", "nodepool", buildLogsHttpdPortName, "/builds", - buildLogsHttpdPort, map[string]string{ - "haproxy.router.openshift.io/rewrite-target": "/dib", - }, r.cr.Spec.FQDN, r.cr.Spec.LetsEncrypt) + buildLogsHttpdPort, map[string]string{}, r.cr.Spec.FQDN, r.cr.Spec.LetsEncrypt) var isReady = r.IsStatefulSetReady(¤t) && routeReady && pvcReadiness diff --git a/controllers/static/nodepool/generate-config.sh b/controllers/static/nodepool/generate-config.sh index 24a64434..d3bcd1fa 100644 --- a/controllers/static/nodepool/generate-config.sh +++ b/controllers/static/nodepool/generate-config.sh @@ -22,6 +22,7 @@ zookeeper-tls: key: /tls/client/tls.key # images-dir is mandatory key for nodepool-builder process images-dir: /var/lib/nodepool/dib +build-log-dir: /var/lib/nodepool/builds/logs EOF if [ "$CONFIG_REPO_SET" == "TRUE" ]; then diff --git a/controllers/static/nodepool/httpd-build-logs-dir.conf b/controllers/static/nodepool/httpd-build-logs-dir.conf index bffa435f..3df06ad5 100644 --- a/controllers/static/nodepool/httpd-build-logs-dir.conf +++ b/controllers/static/nodepool/httpd-build-logs-dir.conf @@ -1,7 +1,7 @@ AddType text/plain .log - + Options Indexes SymLinksIfOwnerMatch Require all granted IndexOptions FancyIndexing HTMLTable NameWidth=* SuppressDescription diff --git a/controllers/utils.go b/controllers/utils.go index cafb3df9..6a4071a3 100644 --- a/controllers/utils.go +++ b/controllers/utils.go @@ -239,14 +239,30 @@ func (r *SFUtilContext) ensureRoute(route apiroutev1.Route, name string) bool { return false } else { // Route already exist - check if we need to update the Route - // Use the String repr of the RouteSpec to compare for changes + needUpdate := false + + // First check the route annotations + if (len(route.Annotations) == 0 && len(current.Annotations) != 0) || (len(route.Annotations) != 0 && len(current.Annotations) == 0) { + current.Annotations = route.Annotations + needUpdate = true + } + if len(route.Annotations) != 0 && len(current.Annotations) != 0 { + if !utils.MapEquals(&route.Annotations, ¤t.Annotations) { + current.Annotations = route.Annotations + needUpdate = true + } + } + + // Use the String repr of the RouteSpec to compare for Spec changes // This comparaison mechanics may fail in case of some Route Spec default values // not specified in the wanted version. - wantedRepr := route.Spec.String() - currentRepr := current.Spec.String() - if wantedRepr != currentRepr { - r.log.V(1).Info("Updating route...", "name", name) + if route.Spec.String() != current.Spec.String() { current.Spec = route.Spec + needUpdate = true + } + + if needUpdate { + r.log.V(1).Info("Updating route...", "name", name) r.UpdateR(¤t) return false } diff --git a/roles/health-check/check-service-uri/tasks/main.yaml b/roles/health-check/check-service-uri/tasks/main.yaml index c08ca4bc..a460ffae 100644 --- a/roles/health-check/check-service-uri/tasks/main.yaml +++ b/roles/health-check/check-service-uri/tasks/main.yaml @@ -89,7 +89,7 @@ - name: Attempt to access Nodepool builder build logs ansible.builtin.uri: - url: "https://{{ nodepool_host }}/builds/" + url: "https://{{ nodepool_host }}/builds" method: GET return_content: true validate_certs: "{{ validate_certs }}" @@ -97,5 +97,5 @@ register: this until: - this.status == 200 - - "'Index of /dib' in this.content" + - "'Index of /builds' in this.content" retries: "{{ retries }}" diff --git a/roles/health-check/config-update-nodepool-builder/tasks/main.yaml b/roles/health-check/config-update-nodepool-builder/tasks/main.yaml index 8c805598..1910ebaa 100644 --- a/roles/health-check/config-update-nodepool-builder/tasks/main.yaml +++ b/roles/health-check/config-update-nodepool-builder/tasks/main.yaml @@ -135,7 +135,7 @@ - name: "Check that witness message is available in build logs" ansible.builtin.shell: | - kubectl exec nodepool-builder-0 -- bash -c "grep 'Hello from build of {{ nodepool_diskimage_name }}' /var/log/nodepool/builds/*" + kubectl exec nodepool-builder-0 -- bash -c "grep 'Hello from build of {{ nodepool_diskimage_name }}' /var/lib/nodepool/builds/logs/*" register: witness until: witness is success delay: 5 diff --git a/roles/post/get-k8s-resources/tasks/main.yaml b/roles/post/get-k8s-resources/tasks/main.yaml index 0836220d..a85482ee 100644 --- a/roles/post/get-k8s-resources/tasks/main.yaml +++ b/roles/post/get-k8s-resources/tasks/main.yaml @@ -33,6 +33,7 @@ - configmaps - jobs - pvc + - routes - csvs - installplans - sub diff --git a/roles/post/get-nodepool-builds-logs/tasks/main.yaml b/roles/post/get-nodepool-builds-logs/tasks/main.yaml index 8d289d14..54ee8af0 100644 --- a/roles/post/get-nodepool-builds-logs/tasks/main.yaml +++ b/roles/post/get-nodepool-builds-logs/tasks/main.yaml @@ -4,7 +4,7 @@ path: "{{ output_dir }}" state: absent -- name: "Fetch nodepool-builder /var/log/nodepool/builds content locally in {{ output_dir }}" +- name: "Fetch nodepool-builder /var/lib/nodepool/builds/logs content locally in {{ output_dir }}" ansible.builtin.command: | - kubectl cp nodepool-builder-0:/var/log/nodepool/builds -c nodepool-builder {{ output_dir }} + kubectl cp nodepool-builder-0:/var/lib/nodepool/builds/logs -c nodepool-builder {{ output_dir }} failed_when: false