-
Notifications
You must be signed in to change notification settings - Fork 43
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
Add support for incremental stamping #175
Conversation
skylib/kustomize/kustomize.bzl
Outdated
variables = "--variable=NAMESPACE={namespace}".format( | ||
namespace = namespace, | ||
) | ||
variables += " --variable=GIT_REVISION=\"$(git rev-parse HEAD)\"" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should not do stamping of stamps in .show
and .apply
commands.
- The git command will not work because the current directory is not the source directory.
- We will be breaking the idempotency properties of
.show
andapply
commands, which is no backward compatible behavior change.
gitops/prer/create_gitops_prs.go
Outdated
dryRun = flag.Bool("dry_run", false, "Do not create PRs, just print what would be done") | ||
stamp = flag.Bool("stamp", false, "Stamp results of gitops targets with volatile information") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dryRun = flag.Bool("dry_run", false, "Do not create PRs, just print what would be done") | |
stamp = flag.Bool("stamp", false, "Stamp results of gitops targets with volatile information") | |
stamp = flag.Bool("stamp", false, "Stamp results of gitops targets with volatile information") | |
dryRun = flag.Bool("dry_run", false, "Do not create PRs, just print what would be done") |
Historically we kept dryRun
as a last argument definition.
gitops/git/git.go
Outdated
} | ||
|
||
// split by newline and ignore empty strings | ||
func SplitFunc(c rune) bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function doesn't seem to belong to the public interface of git
module. Please make it private or inline.
gitops/git/git.go
Outdated
if err != nil { | ||
log.Fatalf("ERROR: %s", err) | ||
} | ||
return strings.FieldsFunc(files, SplitFunc) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer the use of the Scanner
. While it will be more verbose it will be more obvious what empty lines re not returned.
var files []string
sc := bufio.NewScanner(strings.NewReader(s))
for sc.Scan() {
files = append(files, sc.Text())
}
return lines
gitops/git/git.go
Outdated
@@ -103,6 +104,34 @@ func (r *Repo) Commit(message, gitopsPath string) bool { | |||
return true | |||
} | |||
|
|||
// RemoveDiff removes the changes made to a specific file in the repository | |||
func (r *Repo) RemoveDiff(fileName string) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
func (r *Repo) RemoveDiff(fileName string) { | |
func (r *Repo) RestoreFile(fileName string) { |
Align with the terminology used by Git. For example:
Changes not staged for commit:
(use "git add <file>..." to update what will be committed)
(use "git restore <file>..." to discard changes in working directory)
modified: src/index.ts
gitops/prer/create_gitops_prs.go
Outdated
@@ -100,6 +103,40 @@ func bazelQuery(query string) *analysis.CqueryResult { | |||
return qr | |||
} | |||
|
|||
func getContext(workdir *git.Repo, branchName string) map[string]interface{} { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
func getContext(workdir *git.Repo, branchName string) map[string]interface{} { | |
func getGitStatusDict(workdir *git.Repo, branchName string) map[string]interface{} { |
getContext
is too generic.
gitops/prer/create_gitops_prs.go
Outdated
|
||
ctx := getContext(workdir, branchName) | ||
|
||
stampedTemplate := fasttemplate.ExecuteString(string(template), "{{", "}}", ctx) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fasttemplate.Execute
function writes directly into file.
stampedTemplate := fasttemplate.ExecuteString(string(template), "{{", "}}", ctx) | |
outf, err = os.OpenFile(fullPath, os.O_RDWR|os.O_CREATE|os.O_TRUNC, perm) | |
if err != nil { | |
log.Fatalf("Unable to create output file %s: %v", output, err) | |
} | |
defer outf.Close() | |
_, err = fasttemplate.Execute(string(template), "{{", "}}", ctx) |
Description
Modify the
create_gitops_pr
utility to optionally implement the following algorithm:service.yaml
file is an output of theservice.gitops
targetservice.gitops
>service.yaml
service.yaml
digest >service.yaml.digest
service.yaml.digest
are the same as those committed into gitservice.yaml.digest
fileservice.yaml
service.yaml.digest
andservice.yaml
Related Issue
Motivation and Context
The existing GitOps PR process creates a new GitOps PR when a ConfigMap object that exposes a volatile file changes even if the deployment artifact doesn't change.
How Has This Been Tested?
Types of changes
Checklist: