Skip to content

fix: Improve e2e tests and small fixes #6737

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

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

JorTurFer
Copy link
Member

@JorTurFer JorTurFer commented Apr 26, 2025

There is an issue with latest version of the nginx image that we use for e2e tests -> nginx/docker-nginx-unprivileged#302

As the image isn't relevant for the test, this PR pines the image to a working version and uses the GHCR registry instead of dockerhub.

This PR also fixes some small issues on gcp e2e and also remove some wrong error logging on external_push

Checklist

  • Commits are signed with Developer Certificate of Origin (DCO - learn more)

@JorTurFer JorTurFer requested a review from a team as a code owner April 26, 2025 20:34
@JorTurFer
Copy link
Member Author

JorTurFer commented Apr 26, 2025

/run-e2e
Update: You can check the progress here

.
Signed-off-by: Jorge Turrado <[email protected]>
Signed-off-by: Jorge Turrado <[email protected]>
@JorTurFer
Copy link
Member Author

JorTurFer commented Apr 26, 2025

/run-e2e
Update: You can check the progress here

@JorTurFer JorTurFer enabled auto-merge (squash) April 26, 2025 22:26
@JorTurFer
Copy link
Member Author

JorTurFer commented Apr 27, 2025

/run-e2e
Update: You can check the progress here

@JorTurFer
Copy link
Member Author

JorTurFer commented Apr 27, 2025

/run-e2e
Update: You can check the progress here

@zroubalik zroubalik requested a review from Copilot April 29, 2025 05:51
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR pins the nginx image version used in tests to 1.26 and switches from dockerhub (nginxinc) to the GHCR registry. Additionally, there is a change in error handling for the GCP storage scaler.

Reviewed Changes

Copilot reviewed 85 out of 85 changed files in this pull request and generated 1 comment.

File Description
tests/internals/* (all affected test files) Updated nginx image reference to "ghcr.io/nginx/nginx-unprivileged:1.26" for consistency across tests
pkg/scalers/gcp_storage_scaler.go Modified error handling in the getItemCount loop

@JorTurFer
Copy link
Member Author

JorTurFer commented Apr 29, 2025

/run-e2e
Update: You can check the progress here

@JorTurFer
Copy link
Member Author

JorTurFer commented Apr 29, 2025

/run-e2e
Update: You can check the progress here

.
Signed-off-by: Jorge Turrado <[email protected]>
@JorTurFer
Copy link
Member Author

@zroubalik , I have updated external_push scaler to not print error on io.EOF as it's valid response, PTAL too

@JorTurFer
Copy link
Member Author

JorTurFer commented Apr 29, 2025

/run-e2e
Update: You can check the progress here

Signed-off-by: Jorge Turrado <[email protected]>
Signed-off-by: Jorge Turrado <[email protected]>
@JorTurFer
Copy link
Member Author

JorTurFer commented Apr 29, 2025

/run-e2e
Update: You can check the progress here

Signed-off-by: Jorge Turrado <[email protected]>
@JorTurFer JorTurFer changed the title fix: Use pinned version for nginx image fix: Improve e2e tests Apr 29, 2025
@JorTurFer
Copy link
Member Author

JorTurFer commented Apr 29, 2025

/run-e2e
Update: You can check the progress here

@JorTurFer JorTurFer requested a review from Copilot April 29, 2025 22:22
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR improves the e2e test stability by pinning the nginx image to a known good version from the GHCR registry and updating related error logging and connection retry handling. It updates the image references in multiple test files and makes a minor refactor in the GCP storage scaler to change the error handling order.

Reviewed Changes

Copilot reviewed 86 out of 86 changed files in this pull request and generated no comments.

File Description
tests/* Updates image references from "nginxinc/nginx-unprivileged" to "ghcr.io/nginx/nginx-unprivileged:1.26" (with or without alpine-slim) across different e2e test definitions
pkg/scalers/gcp_storage_scaler.go Adjusts the error handling order when iterating over bucket items to pin error conditions
pkg/scalers/external_scaler.go Adds improved error logging and back-off logging for retrying connection acquisition
Comments suppressed due to low confidence (2)

tests/internals/pause_scaledobject_explicitly/pause_scaledobject_explicitly_test.go:60

  • [nitpick] Some tests update the image from the 'alpine-slim' variant to a non-slim version. Please confirm that the change from 'alpine-slim' to '1.26' is intentional and does not affect test behavior.
-          image: nginxinc/nginx-unprivileged:alpine-slim

pkg/scalers/gcp_storage_scaler.go:207

  • In the updated error branch, the code accesses 'item.Size' after an error is detected. This may lead to undefined behavior if 'item' is not valid. Consider checking for 'iterator.Done' or other errors before accessing any fields on 'item'.
if err != nil {

@JorTurFer JorTurFer changed the title fix: Improve e2e tests fix: Improve e2e tests and small fixes Apr 30, 2025
@wozniakjan wozniakjan mentioned this pull request Apr 30, 2025
17 tasks
@JorTurFer
Copy link
Member Author

JorTurFer commented Apr 30, 2025

/run-e2e
Update: You can check the progress here

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