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
Adding a commandline graph pruner #253
Adding a commandline graph pruner #253
Conversation
/assign @smarterclayton |
/hold This shouldn't be merged until the secret has been pruned and the latest state gets saved. |
00281ca
to
d7c79ad
Compare
Squash the code review changes and then this lgtm |
13a8fcb
to
0fb314e
Compare
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bradmwilliams, smarterclayton The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
New changes are detected. LGTM label has been removed. |
f8f6ed9
to
d050f71
Compare
d050f71
to
f82f626
Compare
/unhold |
f82f626
to
79f31e9
Compare
79f31e9
to
ed5d041
Compare
ed5d041
to
c4a56c4
Compare
cmd/release-controller/upgrades.go
Outdated
|
||
if confirm { | ||
buf := &bytes.Buffer{} | ||
saveUpgradeGraph(buf, c.graph, secretClient, ns, name) |
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.
Why are you passing a buf into this method here?
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.
The original logic creates the buffer and then resets it for each iteration of the wait.Until()
loop:
release-controller/cmd/release-controller/upgrades.go
Lines 328 to 349 in 3648252
// keep the secret up to date | |
buf := &bytes.Buffer{} | |
wait.Until(func() { | |
buf.Reset() | |
if err := graph.Save(buf); err != nil { | |
klog.Errorf("Unable to calculate graph state: %v", err) | |
return | |
} | |
secret, err := secretClient.Get(context.TODO(), name, metav1.GetOptions{}) | |
if err != nil { | |
klog.Errorf("Can't read latest secret %s/%s: %v", ns, name, err) | |
return | |
} | |
if secret.Data == nil { | |
secret.Data = make(map[string][]byte) | |
} | |
secret.Data["latest"] = buf.Bytes() | |
if _, err := secretClient.Update(context.TODO(), secret, metav1.UpdateOptions{}); err != nil { | |
klog.Errorf("Can't save state to secret %s/%s: %v", ns, name, err) | |
} | |
klog.V(2).Infof("Saved upgrade graph state to %s/%s", ns, name) | |
}, 5*time.Minute, stopCh) |
When I refactored the code, I extracted the "save" logic into saveUpgradeGraph()
and to preserve the original logic, I passed in the *bytes.Buffer
.
1d99435
to
11078c3
Compare
/remove-lifecycle stale |
54d15a8
to
0921282
Compare
/hold |
49463d9
to
684b2f9
Compare
a7e1aa2
to
927315f
Compare
927315f
to
d3f97aa
Compare
Extracting the load/save logic into their own functions Updating commandline options for pruning graph Code review changes Adding items that are in both the To and From lists Go formatting Performing prune inside method with defer Returning error from saveUpgradeGraph() Post-rebase updates Small refactoring Fix klog version Convert to using set
d3f97aa
to
7012ece
Compare
/unhold |
@bradmwilliams: all tests passed! Full PR test history. Your PR dashboard. 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/test-infra repository. I understand the commands that are listed here. |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: AlexNPavel, bradmwilliams, smarterclayton The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Bumping to version 4.11 by default
Adding a couple commandline options to prune, and verify, the release-upgrade-graph. Calling the release-controller like:
./release-controller --release-namespace ocp --job-namespace ci --prune-graph
, will fetch all the tags from therelease
imagestream. Then it finds the tags that are NOT present in either of theTO
andFROM
upgrade graphs. Then, it generates the union of the 2 lists, and prunes the graph accordingly. Lastly, by default, it will pretty print the resultant graph for verification. Optionally, you can also specify the--print-secret-payload
option to print out the base64 output of the compressed graph.