[feat i2g]add in-cluster console for model comparison#4728
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: shuqz The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4728 +/- ##
==========================================
+ Coverage 56.09% 56.52% +0.43%
==========================================
Files 388 394 +6
Lines 30932 31381 +449
==========================================
+ Hits 17352 17739 +387
- Misses 12566 12607 +41
- Partials 1014 1035 +21 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Unknown CLA label state. Rechecking for CLA labels. Send feedback to sig-contributor-experience at kubernetes/community. /check-cla |
| r.logger.Info("successfully built model", "model", stackJSON) | ||
|
|
||
| if r.featureGates.Enabled(config.IngressPlanAnnotation) && len(ingGroup.Members) > 0 { | ||
| if err := patchDryRunPlanAnnotation(ctx, r.k8sClient, ingGroup.Members[0].Ing, stackJSON); err != nil { |
There was a problem hiding this comment.
it is always patched to members[0] for group ingress. members in a group is sorted by group order. if group order is not specified, it is sorted by lexical order based on namespacedName. i think it is deterministic. and even if it changed, we will still pick up the new members[0] to write annotation and cleanup stale annotations (see line 239 below). as for how it is been discovered from gateway, gateway will list all ingress from a group and find one with dry run plan annotation.
let me know if you still prefer to write it to oldest ingress
| These warnings are informational — the tool still generates output. For the most accurate results, use `--from-cluster` which automatically fetches all referenced resources. | ||
|
|
||
| ## Migration Console (Web UI) | ||
|
|
There was a problem hiding this comment.
i am going to create a new page for console, including screenshot from console as example and instructions to use. but just make it short here
| @@ -0,0 +1,112 @@ | |||
| package console | |||
There was a problem hiding this comment.
please review this file. this is where we define which changes are "Expected"
| const escapeHTML = (s) => String(s).replace(/[&<>"']/g, c => ( | ||
| { '&': '&', '<': '<', '>': '>', '"': '"', "'": ''' }[c] | ||
| )); |
There was a problem hiding this comment.
Do any other characters need to be escaped? Ex. =, /
| `<div class="error-msg">${escapeHTML(gw.error)}</div>`); | ||
| return; | ||
| } | ||
| document.querySelectorAll('.error-msg').forEach(e => e.remove()); |
There was a problem hiding this comment.
Why do we want to remove the error messages?
There was a problem hiding this comment.
clears stale error messages from the DOM when navigating
| .gw-rail-label { | ||
| display: none; | ||
| align-items: center; | ||
| padding: 0 16px; |
There was a problem hiding this comment.
There's a lot of "magic numbers" in the CSS. Could you also define some of the more common ones (like 12px, 16px, etc.) in :root, similar to what you did for some of the more common colors so it's clear why they're set at those sizes?
| <!-- TOPBAR --> | ||
| <header class="topbar"> | ||
| <a class="brand" href="/" id="brand" aria-label="Home"> | ||
| <span class="brand-wordmark">aws<span class="kbd">›</span></span> |
There was a problem hiding this comment.
Nit: Would double check that most class names are readable. Using this as an example, I'm not too sure what kbd represents.
|
|
||
| let target = viewLeft; | ||
| if (tabLeft < viewLeft) { | ||
| target = tabLeft - 12; // small gutter |
There was a problem hiding this comment.
Similar comment about magic numbers, would be good to define these constants somewhere with descriptive names so it's understood why this number in particular is chosen as the gutter size. Also makes adjusting the size easier down the line as adjusting a constant is easier than many instances of 12.
…igs#4734) The IngressPlanAnnotation feature gate, dryrun.go controller logic, and related helm/docs changes are now in a separate PR. This branch retains only the IngressSuffixDryRunPlan constant which the console reads from existing ingress annotations.
Description
/consoleChecklist
README.md, or thedocsdirectory)BONUS POINTS checklist: complete for good vibes and maybe prizes?! 🤯