Conversation
58297b5 to
fcfc129
Compare
|
fcfc129 to
cf1e2b2
Compare
|
Addressed the three review findings in cf1e2b2 (amended into the single commit):
fmt / clippy |
…back `⏎` on a managed resource of an Application that deploys to another cluster only reported where the object lives and told the user to `:ctx` there by hand, then find it again — and the same in reverse to come back. Since nklmilojevic#570 a remote destination resolves to a kubeconfig context, so the jump can be made for them. Pressing `⏎` on such a row switches to the resolved context and, once the connection lands, opens the object as a root view scoped to its name. The kind is resolved against the destination cluster at that point, since the hub may not know the CRD; a kind the destination does not know either falls back to pods with a warning. The deferred target lives in its own slot next to the pending query/bookmark/workspace and follows the same one-at-a-time rules. An unresolved destination still reports where the objects are, and a heading or source line still has nothing to open. The jump records where it came from — the hub context and the object the view was opened for — while the view it opened is on screen. `esc` at that view's root switches back and reopens the Argo CD view on the same object, with the table underneath set to that object's kind. `Tab` through the default kinds keeps the way back (still browsing where the jump landed); a `:resource` query, bookmark or workspace drops it. Rows that jump carry the `→` marker: `draw_findings` takes the jump predicate from the view instead of reading the finding's target, and the Argo CD view marks a managed-resource row whenever its destination resolved to a context. Explain and GitOps keep the target-based rule.
cf1e2b2 to
d9048fa
Compare
|
Follow-up in d9048fa for the locality finding: a resolved context now counts as local only when its kubeconfig server equals the connected |
| if context == self.cluster.context && self.cluster.connected { | ||
| self.flash_warn(&format!( | ||
| "kubeconfig context {context} no longer points at the connected cluster; reconnect with :ctx {context}" | ||
| )); | ||
| return; |
There was a problem hiding this comment.
When kubeconfig remaps the active context name to another server, this branch tells the user to reconnect with :ctx {context}. However, accepting that context calls switch_context, which immediately returns when the requested name matches the connected context. The suggested recovery therefore cannot reconnect to the remapped cluster, leaving the remote managed resource inaccessible until the application is restarted or otherwise disconnected. Provide an in-session path that forces reconnection instead of recommending an operation that is guaranteed to do nothing.
| self.argocd_items | ||
| .get(index) | ||
| .is_some_and(|f| f.target.is_some()) | ||
| || (matches!(self.argocd_destination, Destination::Context(_)) |
There was a problem hiding this comment.
empty resource list still marks none reported with an arrow
| // be a no-op, so say what happened instead of dropping the jump. | ||
| if context == self.cluster.context && self.cluster.connected { | ||
| self.flash_warn(&format!( | ||
| "kubeconfig context {context} no longer points at the connected cluster; reconnect with :ctx {context}" |
There was a problem hiding this comment.
switch_context early returns, so this ctx advice does nothing
| pub(super) struct ArgocdReturn { | ||
| pub context: String, | ||
| pub source: DynamicObject, | ||
| pub kind: crate::k8s::Kind, |
There was a problem hiding this comment.
esc drops the row you left, managed_row_at already exists
There was a problem hiding this comment.
tested live: left row 45, esc returns to row 0
| area: Rect, | ||
| title: String, | ||
| findings: &[crate::explain::Finding], | ||
| jumps: &dyn Fn(usize, &crate::explain::Finding) -> bool, |
There was a problem hiding this comment.
arrow marker is item 5, outside the approved scope
| // The expansion is only discoverable from the title, the way the adjacent | ||
| // view advertises the same key. | ||
| let title = format!(" {} (c discover children) ", app.argocd_title); | ||
| let jumps: Vec<bool> = (0..app.argocd_items.len()) |
There was a problem hiding this comment.
no testbackend test covers this arrow rewiring at all
There was a problem hiding this comment.
reverting the arrow in draw_argocd still passes 1708 tests
| /// The landing half of [`Self::return_to_argocd`]: the table underneath | ||
| /// is the kind the view was opened from, so `esc` out of the view lands | ||
| /// somewhere sensible, and the view itself re-reads its object. | ||
| pub(super) fn reopen_argocd(&mut self, back: ArgocdReturn) { |
There was a problem hiding this comment.
reopen_argocd should restore the managed row, not row zero
|
body says three commits and 1705 tests, both stale |
|
same commit is also open at ShmuelOps#1 |
|
live service-east to apps-dev-east: jump works, return loses place |
mesutoezdil
left a comment
There was a problem hiding this comment.
two bugs, one unfinished round trip, one unapproved item.
none reported rows advertise a jump they cannot make.
the drifted kubeconfig flash advises a ctx switch that early returns.
esc comes back to row zero, verified live at 45 rows deep.
the arrow marker is item 5, and reverting it still passes every test.
nr1 and nr2 otherwise work on a real hub and spoke.
|
I'm also not a fan of that many comments in the code. Which model are you using? Seems that it was low quality output from the get go with so many errors that greptile found from the start. |
Items 1 and 2 of the proposal in #532, as agreed with @mesutoezdil, plus the
→marker that makes the jump discoverable. Three commits, one feature: getting from the Argo CD view to a remote object and back.Problem
Since #570 a remote destination resolves to a kubeconfig context, but
⏎on a managed resource of a remote Application only reports where it lives and tells the user to:ctxthere by hand, then find it again — four steps to look at the pod behind a Degraded row, and the same in reverse to come back.Change
⏎on a remote managed resource jumps to it. Switch to the resolved context and, once the connection lands, open the object as a root view scoped to its name (metadata.name=field selector, same asnavigate_to_target). The kind is resolved against the destination cluster at landing time, since the hub may not know the CRD; a kind the destination does not know either falls back to pods with a warning. The deferred target lives in its own slot next to the pending query / bookmark / workspace and follows the same one-at-a-time rules, cleared on a failed switch.The feat: Argo CD read side - Application columns, :argocd view, fleet count, ownership tree #532 rule "on a remote destination
enterflashes where the object lives instead of opening a same-named object here" guarded against opening the wrong cluster's object when destinations could not be resolved. With fix: resolve Argo CD destination names against every kubeconfig context #570 they resolve to a context, so the guard becomes the jump; an unresolved destination still flashes, and a heading or source line under a remote destination still has nothing to open and says so.esccomes back. The jump records the hub context and the object the view was opened for while the view it opened is on screen.escat that view's root switches back and, once the switch lands, reopens the Argo CD view on the same object, with the table underneath set to that kind.Tabthrough the default kinds in the remote cluster keeps the way back (still browsing where the jump landed); a:resourcequery, bookmark or workspace drops it, soescthere is an ordinary rootesc. The pods fallback keeps it too.→on the rows that now jump.draw_findingstakes the jump predicate from the view instead of reading the finding's target; the Argo CD view marks a managed-resource row whenever its destination resolved to a context. Explain and GitOps keep the target-based rule.No new keys, no new config. Docs:
docs/features.mdArgo CD view paragraph.Tests (through
handle_key)argocd_view_enter_on_a_remote_resource_opens_it_in_its_context— Enter →→on the row → switch armed →land_context→ services scoped to the name →esc→ switch back →Mode::Argocdwith the same title.argocd_view_enter_on_a_remote_heading_does_not_switch,argocd_remote_jump_return_is_dropped_by_a_new_root_view,argocd_remote_jump_return_survives_tab_cycling,argocd_remote_jump_unknown_kind_falls_back_to_pods_and_esc_returns.argocd_view_enter_switches_to_the_alias_context_for_a_remote_name— the fix: resolve Argo CD destination names against every kubeconfig context #570 regression adapted: Enter switches to the alias, not Argo's spelling.argocd_view_will_not_jump_into_a_remote_destination(unresolved) unchanged.cargo fmt --check,cargo clippy --locked --all-targets -- -D warnings,cargo test --locked(1705) pass.