From b045e9be36ab985f91c227522775d317d6fc7c49 Mon Sep 17 00:00:00 2001 From: Ricardo Katz Date: Sun, 21 Apr 2024 18:11:01 -0300 Subject: [PATCH] Do more stuff --- cmd/dataplane/main.go | 5 +++-- deploy/static/provider/kind/deploy.yaml | 7 ++++++- internal/dataplane/nginx/nginx.go | 9 ++++++--- internal/dataplane/nginx/remote.go | 17 ----------------- internal/ingress/controller/nginx.go | 16 +++++++++++++++- pkg/flags/flags.go | 4 ++-- pkg/util/file/file_watcher.go | 21 ++++++++++++++------- rootfs/Dockerfile | 10 +--------- test/e2e/run-kind-e2e.sh | 4 ++-- version/version.go | 12 +++++++++--- 10 files changed, 58 insertions(+), 47 deletions(-) diff --git a/cmd/dataplane/main.go b/cmd/dataplane/main.go index 58698089f..b58337d98 100644 --- a/cmd/dataplane/main.go +++ b/cmd/dataplane/main.go @@ -17,6 +17,7 @@ limitations under the License. package main import ( + "fmt" "net/http" "os" "os/signal" @@ -30,13 +31,13 @@ import ( dataplanenginx "k8s.io/ingress-nginx/cmd/dataplane/pkg/nginx" "k8s.io/ingress-nginx/internal/nginx" "k8s.io/ingress-nginx/pkg/metrics" + "k8s.io/ingress-nginx/version" ) func main() { klog.InitFlags(nil) - //fmt.Println(version.String()) - //var err error + fmt.Println(version.String()) reg := prometheus.NewRegistry() diff --git a/deploy/static/provider/kind/deploy.yaml b/deploy/static/provider/kind/deploy.yaml index abe6faf1b..f9f955596 100644 --- a/deploy/static/provider/kind/deploy.yaml +++ b/deploy/static/provider/kind/deploy.yaml @@ -493,7 +493,7 @@ spec: fieldPath: metadata.namespace - name: LD_PRELOAD value: /usr/local/lib/libmimalloc.so - image: gcr.io/k8s-staging-ingress-nginx/dataplane:v0.0.16 + image: gcr.io/k8s-staging-ingress-nginx/dataplane:v0.0.21 imagePullPolicy: IfNotPresent lifecycle: preStop: @@ -534,6 +534,8 @@ spec: name: ingress-controller - mountPath: /etc/nginx/conf name: nginx-conf + - mountPath: /var/run/secrets/kubernetes.io/serviceaccount + name: emptysecret dnsPolicy: ClusterFirst nodeSelector: ingress-ready: "true" @@ -557,6 +559,9 @@ spec: - name: nginx-conf emptyDir: sizeLimit: 500Mi + - name: emptysecret + emptyDir: + sizeLimit: 1Mi --- apiVersion: batch/v1 kind: Job diff --git a/internal/dataplane/nginx/nginx.go b/internal/dataplane/nginx/nginx.go index ddb592704..585e9bb10 100644 --- a/internal/dataplane/nginx/nginx.go +++ b/internal/dataplane/nginx/nginx.go @@ -7,6 +7,7 @@ import ( "os/exec" "path/filepath" "syscall" + "time" "k8s.io/klog/v2" ) @@ -75,9 +76,7 @@ func (nc NginxCommand) Start(errch chan error) error { return err } } - if err := os.WriteFile(ReadyFile, []byte("OK"), 0644); err != nil { - return err - } + cmd := nc.execCommand(true) cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr @@ -88,6 +87,10 @@ func (nc NginxCommand) Start(errch chan error) error { go func() { errch <- cmd.Wait() }() + now := time.Now().String() + if err := os.WriteFile(ReadyFile, []byte(now), 0644); err != nil { + return err + } return nil } diff --git a/internal/dataplane/nginx/remote.go b/internal/dataplane/nginx/remote.go index 344513270..d300d8ba7 100644 --- a/internal/dataplane/nginx/remote.go +++ b/internal/dataplane/nginx/remote.go @@ -22,23 +22,6 @@ func NewNginxRemote(host string) NginxExecutor { } func (nc NginxRemote) Start(errch chan error) error { - /*getStart, err := url.JoinPath(nc.host, "start") // TODO: Turn this path a constant on dataplane - if err != nil { - return err - } - resp, err := http.Get(getStart) - if err != nil { - return err - } - body, err := io.ReadAll(resp.Body) - if err != nil { - return err - } - if resp.StatusCode != http.StatusOK { - return fmt.Errorf("error executing start: %s", string(body)) - }*/ - - // TODO: Add a ping/watcher to backend and populate error channel return nil } diff --git a/internal/ingress/controller/nginx.go b/internal/ingress/controller/nginx.go index 80879f21e..6bc00daeb 100644 --- a/internal/ingress/controller/nginx.go +++ b/internal/ingress/controller/nginx.go @@ -183,7 +183,21 @@ func NewNGINXController(config *Configuration, mc metric.Collector) *NGINXContro klog.Fatalf("Error creating file watcher for %v: %v", nginx.TemplatePath, err) } - filesToWatch := []string{nginxdataplane.ReadyFile} + // With this test, we will check if the readiness file is not changed. This file is always created or + // changed by dataplane on its start. In case the dataplane dies and restarts, the file will be + // updated with the current time. + // This watcher will then detect a change on the file, meaning controller should restart and reconfigure + // everything. + // It should be guaranteed by the dataplane that this file is changed just once + // NGINX finishes starting + _, err = file.NewFileWatcherUpdateOnly(nginxdataplane.ReadyFile, true, func() { + klog.Fatalf("readiness file changed, restarting contorller") + }) + if err != nil { + klog.Fatalf("error creating file watcher for %v: %v", nginxdataplane.ReadyFile, err) + } + + filesToWatch := []string{} if err := os.Mkdir("/etc/ingress-controller/geoip/", 0o755); err != nil && !os.IsExist(err) { klog.Fatalf("Error creating geoip dir: %v", err) diff --git a/pkg/flags/flags.go b/pkg/flags/flags.go index 5891f636b..f3fd60725 100644 --- a/pkg/flags/flags.go +++ b/pkg/flags/flags.go @@ -261,7 +261,7 @@ https://blog.maxmind.com/2019/12/18/significant-changes-to-accessing-and-using-g parser.EnableAnnotationValidation = *enableAnnotationValidation // check port collisions - if !ing_net.IsPortAvailable(*httpPort) { + /*if !ing_net.IsPortAvailable(*httpPort) { return false, nil, fmt.Errorf("port %v is already in use. Please check the flag --http-port", *httpPort) } @@ -283,7 +283,7 @@ https://blog.maxmind.com/2019/12/18/significant-changes-to-accessing-and-using-g if !ing_net.IsPortAvailable(*profilerPort) { return false, nil, fmt.Errorf("port %v is already in use. Please check the flag --profiler-port", *profilerPort) - } + }*/ nginx.StatusPort = *statusPort nginx.StreamPort = *streamPort diff --git a/pkg/util/file/file_watcher.go b/pkg/util/file/file_watcher.go index 3899e41f8..fffd5e4fe 100644 --- a/pkg/util/file/file_watcher.go +++ b/pkg/util/file/file_watcher.go @@ -37,16 +37,23 @@ type OSFileWatcher struct { watcher *fsnotify.Watcher // onEvent callback to be invoked after the file being watched changes onEvent func() + updateOnly bool +} + +// NewFileWatcher creates a new FileWatcher +func NewFileWatcherUpdateOnly(file string, updateOnly bool, onEvent func()) (Watcher, error) { + fw := OSFileWatcher{ + file: file, + onEvent: onEvent, + updateOnly: updateOnly, + } + err := fw.watch() + return fw, err } // NewFileWatcher creates a new FileWatcher func NewFileWatcher(file string, onEvent func()) (Watcher, error) { - fw := OSFileWatcher{ - file: file, - onEvent: onEvent, - } - err := fw.watch() - return fw, err + return NewFileWatcherUpdateOnly(file, false, onEvent) } // Close ends the watch @@ -72,7 +79,7 @@ func (f *OSFileWatcher) watch() error { for { select { case event := <-watcher.Events: - if event.Has(fsnotify.Create) || + if (!f.updateOnly && event.Has(fsnotify.Create)) || event.Has(fsnotify.Write) { if finfo, err := os.Lstat(event.Name); err != nil { log.Printf("can not lstat file: %v\n", err) diff --git a/rootfs/Dockerfile b/rootfs/Dockerfile index 16c998282..0b167625f 100644 --- a/rootfs/Dockerfile +++ b/rootfs/Dockerfile @@ -39,7 +39,7 @@ RUN apk update \ && adduser -S -D -H -u 101 -h /usr/local/nginx \ -s /sbin/nologin -G www-data -g www-data www-data -COPY --chown=www-data:www-data etc /etc +COPY --chown=www-data:www-data etc/nginx/template /etc/nginx/template COPY --chown=www-data:www-data bin/${TARGETARCH}/dbg / COPY --chown=www-data:www-data bin/${TARGETARCH}/nginx-ingress-controller / @@ -54,9 +54,6 @@ RUN bash -xeu -c ' \ /etc/ingress-controller/geoip \ /etc/ingress-controller/telemetry \ /etc/ingress-controller/tempconf \ - /etc/nginx/conf \ - /var/log \ - /var/log/nginx \ /tmp/nginx \ ); \ for dir in "${writeDirs[@]}"; do \ @@ -74,9 +71,4 @@ RUN apk add --no-cache libcap \ && setcap -v cap_net_bind_service=+ep /nginx-ingress-controller \ && apk del libcap USER www-data - -# Create symlinks to redirect nginx logs to stdout and stderr docker log collector -RUN ln -sf /dev/stdout /var/log/nginx/access.log \ - && ln -sf /dev/stderr /var/log/nginx/error.log - ENTRYPOINT ["/nginx-ingress-controller"] diff --git a/test/e2e/run-kind-e2e.sh b/test/e2e/run-kind-e2e.sh index 891e5dbae..e41f5c28b 100755 --- a/test/e2e/run-kind-e2e.sh +++ b/test/e2e/run-kind-e2e.sh @@ -88,7 +88,7 @@ if [ "${SKIP_INGRESS_IMAGE_CREATION}" = "false" ]; then make BASE_IMAGE="${NGINX_BASE_IMAGE}" -C "${DIR}"/../../ clean-image build image-chroot docker tag ${REGISTRY}/controller-chroot:${TAG} ${REGISTRY}/controller:${TAG} else - make BASE_IMAGE="${NGINX_BASE_IMAGE}" -C "${DIR}"/../../ clean-image build image + make BASE_IMAGE="${NGINX_BASE_IMAGE}" -C "${DIR}"/../../ build image image-dataplane fi echo "[dev-env] .. done building controller images" @@ -111,6 +111,6 @@ KIND_WORKERS=$(kind get nodes --name="${KIND_CLUSTER_NAME}" | grep worker | awk echo "[dev-env] copying docker images to cluster..." kind load docker-image --name="${KIND_CLUSTER_NAME}" --nodes="${KIND_WORKERS}" nginx-ingress-controller:e2e -kind load docker-image --name="${KIND_CLUSTER_NAME}" --nodes="${KIND_WORKERS}" "${REGISTRY}"/controller:"${TAG}" +kind load docker-image --name="${KIND_CLUSTER_NAME}" --nodes="${KIND_WORKERS}" "${REGISTRY}"/controller:"${TAG}" "${REGISTRY}"/dataplane:"${TAG}" echo "[dev-env] running e2e tests..." make -C "${DIR}"/../../ e2e-test diff --git a/version/version.go b/version/version.go index 58e7ca175..2e2067db8 100644 --- a/version/version.go +++ b/version/version.go @@ -33,12 +33,18 @@ var ( // String returns information about the release. func String() string { + ngxVer := nginx.Version() + controllerType := "controller" + if ngxVer != "N/A" { + ngxVer = fmt.Sprintf("NGINX: %s", ngxVer) + controllerType = "dataplane" + } return fmt.Sprintf(`------------------------------------------------------------------------------- -NGINX Ingress controller +NGINX Ingress %s Release: %v Build: %v Repository: %v - %v + %s ------------------------------------------------------------------------------- -`, RELEASE, COMMIT, REPO, nginx.Version()) +`, controllerType, RELEASE, COMMIT, REPO, ngxVer) }