Skip to content

ci: add Docker credentials env vars and skip test if not set#4687

Merged
gaius-qi merged 1 commit intomainfrom
feature/ci
Mar 30, 2026
Merged

ci: add Docker credentials env vars and skip test if not set#4687
gaius-qi merged 1 commit intomainfrom
feature/ci

Conversation

@gaius-qi
Copy link
Copy Markdown
Member

Description

This pull request improves the way Docker credentials are handled in both the CI workflow and the image preheat unit tests. The main changes ensure that Docker Hub credentials are securely passed to the test environment and that tests requiring authentication are skipped if credentials are missing.

CI/CD and Testing Improvements:

  • The CI workflow (.github/workflows/ci.yml) now sets DOCKER_USERNAME and DOCKER_PASSWORD environment variables for the unit test step, allowing tests to authenticate with Docker Hub.

  • The image preheat unit test (internal/job/image_test.go) now reads Docker credentials from environment variables and skips tests if they are not set, preventing authentication errors in environments without secrets configured.

  • The ManifestRequest objects in the test cases are updated to include the Docker credentials, enabling authenticated requests to Docker Hub during testing. [1] [2]

Related Issue

Motivation and Context

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation Update (if none of the other choices apply)

Checklist

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.

Signed-off-by: Gaius <gaius.qi@gmail.com>
@gaius-qi gaius-qi added this to the v2.5.0 milestone Mar 30, 2026
@gaius-qi gaius-qi self-assigned this Mar 30, 2026
@gaius-qi gaius-qi requested review from a team as code owners March 30, 2026 13:15
@gaius-qi gaius-qi added the enhancement New feature or request label Mar 30, 2026
@gaius-qi gaius-qi enabled auto-merge (squash) March 30, 2026 13:15
@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 30, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 28.12%. Comparing base (eda0bdc) to head (c51d991).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4687      +/-   ##
==========================================
- Coverage   28.66%   28.12%   -0.55%     
==========================================
  Files         232      232              
  Lines       23097    23097              
==========================================
- Hits         6620     6495     -125     
- Misses      16015    16161     +146     
+ Partials      462      441      -21     
Flag Coverage Δ
unittests 28.12% <ø> (-0.55%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Member

@EvanCley EvanCley left a comment

Choose a reason for hiding this comment

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

lgtm

@gaius-qi gaius-qi disabled auto-merge March 30, 2026 14:55
@gaius-qi gaius-qi merged commit 4625771 into main Mar 30, 2026
19 of 23 checks passed
@gaius-qi gaius-qi deleted the feature/ci branch March 30, 2026 14:55
jonakeys pushed a commit to jonakeys/dragonfly that referenced this pull request Mar 30, 2026
…lyoss#4687)

Signed-off-by: Gaius <gaius.qi@gmail.com>
Signed-off-by: Jonathan van der Steege <jonathan@jonakeys.nl>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants