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.

Sign up for GitHub

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

Jump to bottom

ocm describe cluster fails for Archived/Deprovisioned clusters #517

Open
1 of 6 tasks
cben opened this issue Jul 4, 2023 · 5 comments
Open
1 of 6 tasks

ocm describe cluster fails for Archived/Deprovisioned clusters #517

cben opened this issue Jul 4, 2023 · 5 comments

Comments

@cben
Copy link
Contributor

cben commented Jul 4, 2023 •
edited
Loading

here is an example Deprovisioned subscription

> ocm get sub 2QaydcSjzvQGWHqxRpSReT7mWW4
{
  ...
  "cluster_id": "24301dug094uiivh57o0hfdhotp1cbom",
  "display_name": "celia-imdsv2",
  "external_cluster_id": "89ffcb09-33bd-47ce-aa21-e793383b136d",
  "href": "/api/accounts_mgmt/v1/subscriptions/2QaydcSjzvQGWHqxRpSReT7mWW4",
  "id": "2QaydcSjzvQGWHqxRpSReT7mWW4",
  "kind": "Subscription",
  "managed": true,
  ...
  "organization_id": "1wuVGGV6SCmD8ya6yRGEJzvmVuC",
  "plan": {
    "href": "/api/accounts_mgmt/v1/plans/MOA",
    ...
  },
  "provenance": "Provisioning",
  "status": "Deprovisioned",
}

and now let's try to ocm describe cluster it:

> ocm describe cluster 89ffcb09-33bd-47ce-aa21-e793383b136d
Error: Can't retrieve cluster for key '89ffcb09-33bd-47ce-aa21-e793383b136d': There are no subscriptions or clusters with identifier or name '89ffcb09-33bd-47ce-aa21-e793383b136d'

> ocm describe cluster celia-imdsv2
Error: Can't retrieve cluster for key 'celia-imdsv2': There are no subscriptions or clusters with identifier or name 'celia-imdsv2'

> ocm describe cluster 2QaydcSjzvQGWHqxRpSReT7mWW4
Error: Can't retrieve cluster for key '2QaydcSjzvQGWHqxRpSReT7mWW4': There are no subscriptions or clusters with identifier or name '2QaydcSjzvQGWHqxRpSReT7mWW4'

> ocm describe cluster 24301dug094uiivh57o0hfdhotp1cbom
Error: Can't retrieve cluster for key '24301dug094uiivh57o0hfdhotp1cbom': There are no subscriptions or clusters with identifier or name '24301dug094uiivh57o0hfdhotp1cbom'

These messages are both misleading technically (there is a subscription with these ids) and unfortunate ☹️.

Here are the accessed APIs:

Request URL is 'https://api.stage.openshift.com/api/accounts_mgmt/v1/subscriptions?search=%28display_name+%3D+%2724301dug094uiivh57o0hfdhotp1cbom%27+or+cluster_id+%3D+%2724301dug094uiivh57o0hfdhotp1cbom%27+or+external_cluster_id+%3D+%2724301dug094uiivh57o0hfdhotp1cbom%27%29+and+status+in+%28%27Reserved%27%2C+%27Active%27%29&size=1'

Request URL is 'https://api.stage.openshift.com/api/clusters_mgmt/v1/clusters?search=id+%3D+%2724301dug094uiivh57o0hfdhotp1cbom%27+or+name+%3D+%2724301dug094uiivh57o0hfdhotp1cbom%27+or+external_id+%3D+%2724301dug094uiivh57o0hfdhotp1cbom%27&size=1'

clusters API indeed stops returning data on Archived/Deprovisioned clusters, but subscriptions API retains a lot of info that could be shown (as UI does on https://console.redhat.com/openshift/archived).

Peeking at GetCluster() helper code, it does query subscriptions but appends status in ('Reserved', 'Active') clause.

  • It's used by many commands, most of which still want to only deal with active managed clusters. For those, the error message should be adjusted to say a subscription exists but is inactive.
    => improve GetCluster message when Subscription exists but is inactive #518
  • rename GetCluster -> GetActiveCluster?
  • GetCluster helper returns a cmv1.Cluster so would be unsuitable anyway when we only have subscription data.
    => Add a new GetSubscription / GetSubscriptionAndCluster helper?
  • PrintClusterDescription would need many adjustments to deal with partial data...
  • Accept subscription IDs (currently only looks up by cluster ID | external UUID | display_name)
  • --output and --json flags now dump Cluster JSON. How can we extend them to include Subscription too / support cases of having only Subscription?
cben added a commit to cben/ocm-cli that referenced this issue Jul 4, 2023
@cben
improve GetCluster message when Subscription exists but is inactive
8f622a1 
First step towards openshift-online#517.
At least doesn't lie to user and gives hint how to find past data:

Error: Can't retrieve cluster for key '0e68a621-c00e-4bbf-a4bb-9915ae6cc947': Cluster was Deprovisioned, see `ocm get subscription 2S434jffVG4fSgDdK4cbkDTxVNp` for details

This affects ALL commands that use `GetCluster` helper.

Behavior change: when there is 1 active cluster and also 1+ past clusters
reusing same display_name, previously the active one was used;
now will refuse treating them as ambiguous:

> ocm get subs -p search="display_name in ('servheredia')" | jq .items[].status | sort | uniq -c
      4 "Deprovisioned"
      1 "Reserved"
> ocm describe cluster servheredia
Error: Can't retrieve cluster for key 'servheredia': There are 5 subscriptions with cluster identifier or name 'servheredia'
@cben cben mentioned this issue Jul 4, 2023
Merged
@cben
Copy link
Contributor Author

cben commented Dec 13, 2023 •
edited
Loading

So my #518 caused some complaints, at least from 2 internal users, when reusing a display_name of previously deleted clusters 👎

Scenarios:

  1. The key matches zero clusters — no change.
  2. The key matches exactly 1 deleted cluster 🗑️.
    This is what improve GetCluster message when Subscription exists but is inactive #518 wanted to improve, by telling you e.g. Cluster was Deprovisioned, see `ocm get subscription 2S434jffVG4fSgDdK4cbkDTxVNp` for details
  3. The key matches >1 cluster, all deleted 🗑️🗑️🗑️.
    The error message that There are %d subscriptions with cluster identifier or name '%s' is (1) technically correct but hardly actionable, because it lists no ids, and ocm list clusters won't even show deleted clusters.
  4. The key matches exactly 1 active cluster — no change.
  5. The key matches 1 active cluster + 1 or more deleted ✔️🗑️🗑️
    This is the UX regression 👎: Previously ocm commands would just silently use the active one, now they exit with error.
    User can still proceed by doing ocm list clusters NAME, and passing exact ID instead of name.
  6. The key matches >1 active clusters ✔️✔️?! (A) Unlikely but technically possible if you set one cluster's display_name to equal another's ID (B) If you use undocumented percents e.g. %foo%.
    No change. (This is the ambiguity subsTotal > 1 check was intended to guard from.)

=> So I'm gonna revert it (maybe tweaking the "not found" wording a little).

Luckily, both old and new GetCluster() logic only allow "Reserved" and "Active" cluster:

ocm-cli/pkg/cluster/cluster.go

Lines 199 to 200 in 6d866ac

if !ok || (status != "Reserved" && status != "Active") {
err = fmt.Errorf("Cluster was %s, see `ocm get subscription %s` for details", status, subID)

There is no functionality yet to actually operate on archived/deprovisioned clusters, so it won't break any command.
Only the message for "matches exactly 1 deleted cluster" will become less informative, but the "1 active + 1 or more deleted" use case is more important.

@cben
Copy link
Contributor Author

cben commented Dec 13, 2023

Huh, #518 may have also broken handling of clusters in some other states. When zero subscriptions matched, it falls back to querying clusters, and if it finds exactly 1 it returns it successfully.

For example IIUC in old logic, a Stale subscription would not match so it'd fall back and the cluster could be returned(?), while with #518 it will find the subscription and error out, not give the fallback a chance 👎
This further supports reverting.

  • need test case for "1 active + 1 or more deleted"
  • need test case for fallback to Cluster lookup

@cben
Copy link
Contributor Author

cben commented Dec 13, 2023 •
edited
Loading

  1. 1 Stale
  2. 1 Stale + 1 or more deleted
  3. >1 Stale
  4. 1 Active + 1 or more Stale
  5. >1 Active + 1 or more Stale
  6. 1 Active + 1 Stale + 1 deleted

@cben
Copy link
Contributor Author

cben commented Dec 14, 2023

Yep, current code refuses to describe a Stale cluster (with unambiguous ID!):

> ocm describe cluster 27p17lo7mpk3ju97ngej0su85stlr6ga
Error: Can't retrieve cluster for key '27p17lo7mpk3ju97ngej0su85stlr6ga': Cluster was Stale, see `ocm get subscription 2YklT9XVqpQfRQ0zVNgtd2icIYj` for details

and works with #518 reverted:

> ocm describe cluster 27p17lo7mpk3ju97ngej0su85stlr6ga

ID:			27p17lo7mpk3ju97ngej0su85stlr6ga
External ID:		f6b170a4-13bf-4a89-9f3e-5f977e4109e9
Name:			f6b170a4-13bf-4a89-9f3e-5f977e4109e9
Display Name:		f6b170a4-13bf-4a89-9f3e-5f977e4109e9
State:			ready 
API URL:		
API Listening:		
Console URL:		https://console-openshift-console.apps.spoke-0.qe.lab.redhat.com
Masters:		0
Infra:			0
Computes:		0
Product:		ocp
Subscription type:	
Provider:		baremetal
Version:		4.14.4
Region:			
Multi-az:		false
CCS:			false
HCP:			false
Subnet IDs:		[]
PrivateLink:		false
STS:			false
Existing VPC:		unsupported
Channel Group:		
Cluster Admin:		false
Organization:		Red Hat Inc.
Creator:		mcornea@redhat.com
Email:			mcornea@redhat.com
AccountNumber:          1460290
Created:		2023-11-27T09:31:03Z
Expiration:		0001-01-01T00:00:00Z

@cben
Copy link
Contributor Author

cben commented Jan 10, 2024

  • There are also some weird subscriptions without subscription.status field! 🤷‍♂️
    These now give confusing & contradictory errors: 
    > ocm describe cluster 2CUX7WsBdoXKI9xMSRPXS4s5oLe
Error: Can't retrieve cluster for key '2CUX7WsBdoXKI9xMSRPXS4s5oLe': There are no subscriptions or clusters with identifier or name '2CUX7WsBdoXKI9xMSRPXS4s5oLe'
> ocm describe cluster 1tmtnutmah2arlel7ul65o1efgll2m6i
Error: Can't retrieve cluster for key '1tmtnutmah2arlel7ul65o1efgll2m6i': Cluster was , see `ocm get subscription 2CUX7WsBdoXKI9xMSRPXS4s5oLe` for details
> ocm describe cluster 8f736269-9518-4460-95c3-9a14abc7ebe4
Error: Can't retrieve cluster for key '8f736269-9518-4460-95c3-9a14abc7ebe4': Cluster was , see `ocm get subscription 2CUX7WsBdoXKI9xMSRPXS4s5oLe` for details
    (that subscription does exist! "provenance":"Telemetry", but "last_telemetry_date":"0001-01-01T00:00:00Z". The cluster record also exists, "state":"ready". Hard to say from API what happened to it or how come it has no subscription.status, but the CLI errors are not helping... As a rule, I'd like CLI to reflect API reality, even when that reality is borked.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@cben