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.

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

Make bitbucket server/data center git commit status functionality more robust #701

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

gdasson
Copy link
Contributor

@gdasson gdasson commented Dec 30, 2023

This is an extension to this initial PR

This PR offers the below:

  • Fixes this bug
  • Adds more bitbucket server states:
    • Context: Currently, the code supports only 2 bitbucket server build status states SUCCESSFUL or FAILED based on event severity types info or error respectively.
      For some info events, the bitbucket server build status is incorrectly being set as SUCCESSFUL. For example:
      • an info event with event reason DependencyNotReady should ideally be set as UNKNOWN
      • an info event with event reason Progressing (for ex: Health checks still haven't passed),should ideally be set as INPROGRESS
        This PR adds these functionalities.

Reference: This REST API document shows the different Bitbucket server build status states available for use.

Screenshot to show the different states on Bitbucket Server UI
Screenshot 2023-12-29 at 10 38 16 PM

Signed-off-by: Gaurav Dasson <gaurav.dasson@gmail.com>
Copy link
Member

@stefanprodan stefanprodan left a comment

Choose a reason for hiding this comment

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

Hey @gdasson thanks for this, but we can't make this controller depend on another. I propose you open a new PR with just the bug fix. To handle the reasons, first we need to move them to fluxcd/pkg then we can use those here without adding cross-controller dependencies.

@gdasson
Copy link
Contributor Author

gdasson commented Dec 31, 2023

@stefanprodan thanks for the review and feedback. What is the process to add constants to fluxcd/pkg? Should I initiate a proposal conversation on flux-contributors slack channel? Or just submit a PR on fluxcd/pkg and kustomize-controller describing the proposal of movings constants to fluxcd/pkg? Please let me know. Thx.

@darkowlzz
Copy link
Contributor

What is the process to add constants to fluxcd/pkg? Should I initiate a proposal conversation on flux-contributors slack channel? Or just submit a PR on fluxcd/pkg and kustomize-controller describing the proposal of movings constants to fluxcd/pkg? Please let me know. Thx.

@gdasson It should be fine to just open a PR adding the necessary reasons in https://github.com/fluxcd/pkg/blob/b60db8e44ea93b1a27a05b5083d77d9c56123e9f/apis/meta/conditions.go#L123 along with the other reasons. And then another PR in kustomize-controller using these new constants from pkg repo. We haven't had any formal process for adding such common constant values.

@stefanprodan
Copy link
Member

@gdasson can you please open a separate PR to fix the bug so we can include it in the next patch release? The handling of status conditions can't go into a patch release and these should cover all Git providers not only BitBucket.

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.

None yet

3 participants