Skip to content

Conversation

@feichashao
Copy link
Contributor

Code changes assisted by Claude Code. Reviewed and validated by human.

Problem

When I use the promote saas command, it failed to create a commit.

osdctl promote saas --serviceName saas-xxxx-xxxx --osd --gitHash xxxxx

Error while promoting service: failed to commit changes to app-interface; manual commit may still succeed: failed to commit changes: command failed: exit status 1

The issue was,

  • osdctl reads the current git hash in the current branch.
Current Git Hash: 58384043156bf396ea76e7c6101c0f002145dd57
  • Then, osdctl tries to replace the currently hash in the master branch with the new hash. However, the master branch is out of sync with the upstream branch.
  • My environment also has another corner case. The master branch is not tracking the origin/master.

Changes

  • This PR makes the promote command to sync and use the latest origin/master before reading and replacing the git hash.
Aspect BEFORE AFTER
Sync with remote ❌ Never syncs ✅ Syncs at the very beginning
When sync happens N/A Before reading any files
What gets synced N/A git reset --hard origin/master
File read source Local master (could be outdated) origin/master (always up-to-date)
currentGitHash Could be wrong if master outdated Always correct
String replacement Could fail if hashes mismatch Always works
Promotion baseline Unpredictable Always from origin/master
User experience Must remember to git pull first Automatic sync

@openshift-ci openshift-ci bot requested review from MateSaary and Tafhim February 9, 2026 05:27
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Feb 9, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: feichashao
Once this PR has been reviewed and has the lgtm label, please assign bergmannf for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Feb 9, 2026

@feichashao: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

Copy link
Contributor

@joshbranham joshbranham left a comment

Choose a reason for hiding this comment

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

I think the owness should be to have users keep their checkout clean. Trying to program around that could have some unintended consequences.

If the process fails, how about we just print a helpful message "Please ensure you are on the default branch (master) and are up to date with the remote repository."

// Fetch and sync with origin/master FIRST before reading any files
// to ensure we're working with the latest app-interface state
fmt.Println("Syncing app-interface with origin/master...")
if err := appInterface.GitExecutor.Run(appInterface.GitDirectory, "git", "fetch", "origin", "master"); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

To your point in the PR, what happens if someone's default remote is not "origin"?

if err := appInterface.GitExecutor.Run(appInterface.GitDirectory, "git", "checkout", "master"); err != nil {
return fmt.Errorf("failed to checkout master: %v", err)
}
if err := appInterface.GitExecutor.Run(appInterface.GitDirectory, "git", "reset", "--hard", "origin/master"); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

This could cause unintended consequences if someone has something committed locally.

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