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

Track branches by polling #162

Merged
merged 5 commits into from
Jul 23, 2021
Merged

Track branches by polling #162

merged 5 commits into from
Jul 23, 2021

Conversation

lblackstone
Copy link
Member

@lblackstone lblackstone commented Jul 19, 2021

Proposed changes

Poll the specified git repo every 60 seconds and automatically
deploy the latest commit if it has changed.

Related issues (optional)

Fix #50

@lblackstone lblackstone requested a review from viveklak July 19, 2021 23:50
@lblackstone

This comment has been minimized.

@lblackstone lblackstone force-pushed the lblackstone/branch-tracking branch from d4c12ab to 6c20101 Compare July 20, 2021 23:41
@lblackstone lblackstone marked this pull request as ready for review July 21, 2021 17:17
@lblackstone
Copy link
Member Author

One open question I have is whether we should also track the default (master) branch if no branch or commit is specified. I currently only implemented tracking if a branch is explicitly set.

Alternatively, we could expose a flag to toggle branch tracking, but it seems like the tracking behavior is probably what users would expect if they explicitly opt into a branch rather than a commit.

@vivekpulumi vivekpulumi self-requested a review July 21, 2021 18:25
@vivekpulumi
Copy link

One open question I have is whether we should also track the default (master) branch if no branch or commit is specified. I currently only implemented tracking if a branch is explicitly set.

Alternatively, we could expose a flag to toggle branch tracking, but it seems like the tracking behavior is probably what users would expect if they explicitly opt into a branch rather than a commit.

I think requiring a branch is reasonable. Currently the comments in the stack CR specification are our defacto documentation. Could you update the comments for branch and commit to clarify this behavior: https://github.com/pulumi/pulumi-kubernetes-operator/blob/master/pkg/apis/pulumi/v1alpha1/stack_types.go#L98?

CHANGELOG.md Outdated Show resolved Hide resolved
@vivekpulumi
Copy link

We might also need to call out that every commit is not necessarily reflected by the operator in this mode. e.g. if multiple commits in short succession, the operator will only try to reflect the latest at the time of polling.

@vivekpulumi
Copy link

CC @elsesiy for comments/thoughts.

Copy link
Member

@mikhailshilkov mikhailshilkov left a comment

Choose a reason for hiding this comment

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

I'm not super familiar with the operator but the change LGTM. I guess I'd expend master to work the same way by default but I'll leave this to your judgment.

@lblackstone
Copy link
Member Author

I guess I'd expend master to work the same way by default but I'll leave this to your judgment.

I opened #164 to track the likely resolution to this difference.

@@ -209,6 +209,20 @@ func (r *ReconcileStack) Reconcile(request reconcile.Request) (reconcile.Result,
}
}

// If a branch is specified, then track changes to the branch.
trackBranch := len(sess.stack.Branch) > 0
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason why the change to disable branch tracking if an explicit commit is specified?

Copy link
Member Author

Choose a reason for hiding this comment

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

I realized that branch and commit are mutually exclusive, so we should only need to check for the presence of branch.

// (optional) Commit is the hash of the commit to deploy. If used, HEAD will be in detached mode. This
// is mutually exclusive with the Branch setting. If both are empty, the `master` branch is deployed.
Commit string `json:"commit,omitempty"`
// (optional) Branch is the branch name to deploy, either the simple or fully qualified ref name. This
// is mutually exclusive with the Commit setting. If both are empty, the `master` branch is deployed.
Branch string `json:"branch,omitempty"`

Poll the specified git repo every 60 seconds and automatically
deploy the latest commit if it has changed.
@lblackstone lblackstone force-pushed the lblackstone/branch-tracking branch from d951fb8 to f134617 Compare July 23, 2021 20:45
@lblackstone lblackstone merged commit 0201d3f into master Jul 23, 2021
@lblackstone lblackstone deleted the lblackstone/branch-tracking branch July 23, 2021 21:01
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.

Add support to track Git repo branches by polling / using webhooks to act on branch updates
4 participants