Skip to content

Conversation

@zhogu
Copy link

@zhogu zhogu commented Oct 16, 2025

No description provided.

@zhogu zhogu changed the title Zhogu/cicd build all CI/CD update: add a new workflow to test building all images; skip push specific images in alertmanager to GHCR; Oct 29, 2025
@zhogu zhogu marked this pull request as ready for review October 29, 2025 11:56
Copilot AI review requested due to automatic review settings October 29, 2025 11:56
Copy link

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 introduces workflow improvements and fixes, including special handling for the alert-manager service, standardization of quote styles, and workflow deprecation updates.

  • Removed alert-manager from the build-deploy-changes workflow filter and added custom logic to push only specific alert-manager images to GHCR
  • Added a new workflow to build all services for comprehensive testing
  • Fixed a typo in the Dockerfile chmod command approach and added kusto-sdk to .gitignore

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
.github/workflows/build-deploy-changes.yaml Removed alert-manager filter, added special handling for alter-manager (typo), standardized quote styles, and fixed indentation
.github/workflows/build-all.yaml New workflow to build all services with error handling and reporting
src/cluster-local-storage/build/cluster-local-storage.common.dockerfile Replaced COPY --chmod with separate RUN chmod command
src/alert-manager/.gitignore Added kusto-sdk directory to ignore list
.github/CODEOWNERS Added code ownership rules for the repository

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +124 to +128
# check whether alter-manager is in the changed services
echo "Changed services before removing alter-manager: $changed_services"
if [[ "$changed_services" == *"alter-manager"* ]]; then
echo "alter-manager is in the changed services"
changed_services=$(echo $changed_services | sed 's/alter-manager//g')
Copy link

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

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

The service name is misspelled as 'alter-manager' but should be 'alert-manager' to match the actual service name used elsewhere in the codebase.

Suggested change
# check whether alter-manager is in the changed services
echo "Changed services before removing alter-manager: $changed_services"
if [[ "$changed_services" == *"alter-manager"* ]]; then
echo "alter-manager is in the changed services"
changed_services=$(echo $changed_services | sed 's/alter-manager//g')
# check whether alert-manager is in the changed services
echo "Changed services before removing alert-manager: $changed_services"
if [[ "$changed_services" == *"alert-manager"* ]]; then
echo "alert-manager is in the changed services"
changed_services=$(echo $changed_services | sed 's/alert-manager//g')

Copilot uses AI. Check for mistakes.
run: |
services=$(ls -1d src/* | awk -F'/' '{print $2}' | tr '\n' ' ')
echo "All services: $services"
echo "::set-output name=services::$services"
Copy link

Copilot AI Oct 29, 2025

Choose a reason for hiding this comment

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

The 'set-output' command is deprecated in GitHub Actions. Use the newer syntax: echo "services=$services" >> $GITHUB_OUTPUT instead.

Suggested change
echo "::set-output name=services::$services"
echo "services=$services" >> $GITHUB_OUTPUT

Copilot uses AI. Check for mistakes.
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