Skip to content
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.

Sign up for GitHub

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

Fix push branch reporting when is equal to checkout branch #581

Merged
merged 2 commits into from
Sep 18, 2023
Merged

Fix push branch reporting when is equal to checkout branch #581

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
73466b6
fix push branch reporting if its equal to checkout branch
aryan9600 Sep 1, 2023
db8a257
add tests to check object's status message
aryan9600 Sep 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 11 additions & 22 deletions internal/controller/imageupdateautomation_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,15 +234,18 @@ func (r *ImageUpdateAutomationReconciler) Reconcile(ctx context.Context, req ctr
}
}()

// pushBranch contains the branch name the commit needs to be pushed to.
// It takes the value of the push branch if one is specified or, if the push
// config is nil, then it takes the value of the checkout branch if possible.
var pushBranch string
var switchBranch bool
if gitSpec.Push != nil {
if gitSpec.Push != nil && gitSpec.Push.Branch != "" {
pushBranch = gitSpec.Push.Branch
tracelog.Info("using push branch from .spec.push.branch", "branch", pushBranch)
// We only need to switch branches when a branch has been specified in
// the push spec and it is different than the one in the checkout ref.
if gitSpec.Push.Branch != "" && gitSpec.Push.Branch != checkoutRef.Branch {
pushBranch = gitSpec.Push.Branch
if gitSpec.Push.Branch != checkoutRef.Branch {
switchBranch = true
tracelog.Info("using push branch from .spec.push.branch", "branch", pushBranch)
}
} else {
// Here's where it gets constrained. If there's no push branch
Expand Down Expand Up @@ -402,25 +405,11 @@ func (r *ImageUpdateAutomationReconciler) Reconcile(ctx context.Context, req ctr
pushCtx, cancel := context.WithTimeout(ctx, origin.Spec.Timeout.Duration)
defer cancel()

var pushToBranch bool
var pushWithRefspec bool
// If a refspec is specified, then we need to perform a push using
// that refspec.
if gitSpec.Push != nil && gitSpec.Push.Refspec != "" {
pushWithRefspec = true
}
// We need to push the commit to the push branch if one was specified, or if
// no push config was specified, then we need to push to the branch we checked
// out to.
if (gitSpec.Push != nil && gitSpec.Push.Branch != "") || gitSpec.Push == nil {
pushToBranch = true
}

var pushConfig repository.PushConfig
if gitSpec.Push != nil {
pushConfig.Options = gitSpec.Push.Options
}
if pushToBranch {
if pushBranch != "" {
// If the force push feature flag is true and we are pushing to a
// different branch than the one we checked out to, then force push
// these changes.
Expand All @@ -433,17 +422,17 @@ func (r *ImageUpdateAutomationReconciler) Reconcile(ctx context.Context, req ctr
return failWithError(err)
}
log.Info("pushed commit to origin", "revision", rev, "branch", pushBranch)
statusMessage.WriteString(fmt.Sprintf("commited and pushed commit '%s' to branch '%s'", rev, pushBranch))
statusMessage.WriteString(fmt.Sprintf("committed and pushed commit '%s' to branch '%s'", rev, pushBranch))
}

if pushWithRefspec {
if gitSpec.Push != nil && gitSpec.Push.Refspec != "" {
pushConfig.Refspecs = []string{gitSpec.Push.Refspec}
if err := gitClient.Push(pushCtx, pushConfig); err != nil {
return failWithError(err)
}
log.Info("pushed commit to origin", "revision", rev, "refspec", gitSpec.Push.Refspec)

if pushToBranch {
if statusMessage.Len() > 0 {
statusMessage.WriteString(fmt.Sprintf(" and using refspec '%s'", gitSpec.Push.Refspec))
} else {
statusMessage.WriteString(fmt.Sprintf("committed and pushed commit '%s' using refspec '%s'", rev, gitSpec.Push.Refspec))
Expand Down
24 changes: 23 additions & 1 deletion internal/controller/update_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ func TestImageAutomationReconciler_commitMessage(t *testing.T) {
updateStrategy := &imagev1.UpdateStrategy{
Strategy: imagev1.UpdateStrategySetters,
}
err := createImageUpdateAutomation(testEnv, "update-test", s.namespace, s.gitRepoName, s.gitRepoNamespace, s.branch, "", "", testCommitTemplate, "", updateStrategy)
err := createImageUpdateAutomation(testEnv, "update-test", s.namespace, s.gitRepoName, s.gitRepoNamespace, s.branch, s.branch, "", testCommitTemplate, "", updateStrategy)
g.Expect(err).ToNot(HaveOccurred())

// Wait for a new commit to be made by the controller.
Expand All @@ -225,6 +225,17 @@ func TestImageAutomationReconciler_commitMessage(t *testing.T) {
g.Expect(signature).NotTo(BeNil())
g.Expect(signature.Name).To(Equal(testAuthorName))
g.Expect(signature.Email).To(Equal(testAuthorEmail))

// Regression check to ensure the status message contains the branch name
// if checkout branch is the same as push branch.
imageUpdateKey := types.NamespacedName{
Namespace: s.namespace,
Name: "update-test",
}
var imageUpdate imagev1.ImageUpdateAutomation
_ = testEnv.Get(context.TODO(), imageUpdateKey, &imageUpdate)
ready := apimeta.FindStatusCondition(imageUpdate.Status.Conditions, meta.ReadyCondition)
g.Expect(ready.Message).To(Equal(fmt.Sprintf("committed and pushed commit '%s' to branch '%s'", head.Hash().String(), s.branch)))
},
)
})
Expand Down Expand Up @@ -517,6 +528,17 @@ func TestImageAutomationReconciler_push_refspec(t *testing.T) {
refspecHash := getRemoteRef(g, repoURL, "smth/else")
g.Expect(pushBranchHash.String()).ToNot(Equal(preChangeCommitId))
g.Expect(pushBranchHash.String()).To(Equal(refspecHash.String()))

imageUpdateKey := types.NamespacedName{
Namespace: s.namespace,
Name: "push-refspec",
}
var imageUpdate imagev1.ImageUpdateAutomation
_ = testEnv.Get(context.TODO(), imageUpdateKey, &imageUpdate)
ready := apimeta.FindStatusCondition(imageUpdate.Status.Conditions, meta.ReadyCondition)
g.Expect(ready.Message).To(Equal(
fmt.Sprintf("committed and pushed commit '%s' to branch '%s' and using refspec '%s'",
pushBranchHash.String(), pushBranch, refspec)))
},
)
})
Expand Down
Loading