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.

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

georgettica/login to owner cluster #55

Merged

Conversation

georgettica
Copy link
Contributor

@georgettica georgettica commented Aug 1, 2021

  • feat(ocm-login-owner): login to owner cluster

- get-shard-id extracts the external_id of the parent shard

the mapping is done by hand as there is no actuall mapping between
shard id to the external_id (or any unique identifier for that manner)
once that mapping will exist this automation will use it instead
@georgettica georgettica force-pushed the georgettica/login-to-owner-cluster branch from 4615000 to 4b28583 Compare August 1, 2021 13:08
@clcollins
Copy link
Member

Can you be more descriptive what this is doing? I think I get it reading the code, but login to owner cluster isn't real clear, and I want to make sure I understand.

Regarding the mixed bash and python - I'm not going to say it's enough to downvote the PR, but would this be better to just use python for the whole thing rather than dragging it in at the end?

The code itself looks good, though!

Copy link
Contributor

@T0MASD T0MASD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good and I like this. As @clcollins mentioned, it would be nice to understand how this is used.
if I'm not logged in I'm gettng an error.

Traceback (most recent call last):
  File "/root/./get-shards-clusterid", line 35, in <module>
    described_clusters_raw = convert_to_dict(res)
  File "/root/./get-shards-clusterid", line 12, in convert_to_dict
    return json.loads(stringified_bytes)
  File "//usr/lib64/python3.9/json/__init__.py", line 346, in loads
    return _default_decoder.decode(s)
  File "//usr/lib64/python3.9/json/decoder.py", line 337, in decode
    obj, end = self.raw_decode(s, idx=_w(s, 0).end())
  File "//usr/lib64/python3.9/json/decoder.py", line 355, in raw_decode
    raise JSONDecodeError("Expecting value", s, err.value) from None
json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0)

Maybe run ocm config get url or similar to check if logged in?

utils/bin/get-shards-clusterid Outdated Show resolved Hide resolved
@georgettica
Copy link
Contributor Author

Fixing your comment now @T0MASD

- some functionality was taken from utils/bin/create-cluster
- fix OCM_CONTAINER_DOC to be correct
this is a seperate commit in case the fuctionality is too complex and
should be half bash half python

Co-authored-by: Tomas Dabašinskas <todabasi@redhat.com>
@georgettica georgettica force-pushed the georgettica/login-to-owner-cluster branch from 85806e2 to c09d725 Compare August 3, 2021 13:22
@georgettica
Copy link
Contributor Author

@T0MASD can you look again?

I can add README aswell

@georgettica georgettica requested review from T0MASD and clcollins and removed request for drewandersonnz and rogbas August 4, 2021 08:37
Copy link
Contributor

@T0MASD T0MASD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking nice. I've tried running ./ocm-login-owner <cluster_id> but I get

Error: exec: "xdg-open": executable file not found in $PATH
FAILURE: unable to login

I ran dnf install xdg-utils and re-ran, but then I get a different error:

/usr/bin/xdg-open: line 881: www-browser: command not found
/usr/bin/xdg-open: line 881: links2: command not found
/usr/bin/xdg-open: line 881: elinks: command not found
/usr/bin/xdg-open: line 881: links: command not found
/usr/bin/xdg-open: line 881: lynx: command not found
/usr/bin/xdg-open: line 881: w3m: command not found

utils/bin/get-shards-clusterid Outdated Show resolved Hide resolved
utils/bin/ocm-login-owner Outdated Show resolved Hide resolved
georgettica and others added 2 commits August 5, 2021 12:11
Co-authored-by: Tomas Dabašinskas <todabasi@redhat.com>
Co-authored-by: Tomas Dabašinskas <todabasi@redhat.com>
@georgettica
Copy link
Contributor Author

@T0MASD this PR is mostly to set the groundwork for backplane, I added to ocm-login as this is the easiest way to show how this works

The tool will be added to the downstream once merged

@T0MASD
Copy link
Contributor

T0MASD commented Aug 5, 2021

thanks
/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 5, 2021
@georgettica georgettica merged commit f89f075 into openshift:master Aug 5, 2021
@georgettica georgettica deleted the georgettica/login-to-owner-cluster branch August 5, 2021 11:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants