-
Notifications
You must be signed in to change notification settings - Fork 243
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
Fix oc to odo project translation #6949
Fix oc to odo project translation #6949
Conversation
Signed-off-by: Parthvi <parthvi.vala@gmail.com>
✅ Deploy Preview for odo-docusaurus-preview canceled.
|
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.
Hey @valaparthvi !
Great to see you contributing to odo ;-)
Your changes work great for the issue reported.
However, I noticed a regression when the user already has access to more than one project.
$ odo login -u developer https://api.crc.testing:6443
Connecting to the OpenShift cluster
Logged into "https://api.crc.testing:6443" as "developer" using existing credentials.
You don't have any projects. You can try to create a new project, by running
odo create project <project-name>
$ odo create project test
✓ Creating the project "test" [150ms]
✓ Project "test" is ready for use
✓ New project created and now using project: test
$ odo login -u developer https://api.crc.testing:6443
Connecting to the OpenShift cluster
Logged into "https://api.crc.testing:6443" as "developer" using existing credentials.
You have one project on this server: "test"
Using project "test".
Everything looks good until I create an additional project:
$ odo create project test2
✓ Creating the project "test2" [165ms]
✓ Project "test2" is ready for use
✓ New project created and now using project: test2
$ odo login -u developer https://api.crc.testing:6443
Connecting to the OpenShift cluster
Logged into "https://api.crc.testing:6443" as "developer" using existing credentials.
You have access to the following projects and can switch between them with 'odo project <project-name>':
test
* test2
Using project "test2".
Now, the output displays 'odo project <project-name>'
, which is not accurate. It should display 'odo set project <project-name>'
instead
This is weird. I don't exactly understand why that is so, and I'm unable to debug it (due to the lack of an OC cluster), but I'm going to make some changes, can you see if it works? If not, can you help me debug (when you can find the time) ?
Haha, me too :D |
Signed-off-by: Parthvi <parthvi.vala@gmail.com>
I tried to see if it works with your new changes, but unfortunately the issue still persists:
|
Sure. I was thinking maybe you could add a simple unit test for the
I hope that would help. |
Signed-off-by: Parthvi <parthvi.vala@gmail.com>
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.
Thanks for adding the unit test cases. I was surprised at first sight to see them passing, as the issue with more than one project still persists on my side :-)
After digging a little bit, I narrowed it down to how we configure LoginOptions
. I noticed that the CommandName
field actually changes the output of the login process.
So I guess that's why we are seeing odo project <projectname>
, which did not get replaced by filteredInformation
.
So I think we need to change it to oc
for filtteredInformation
to work properly:
diff --git a/pkg/auth/login.go b/pkg/auth/login.go
index a7909aae8..ee90ce1f7 100644
--- a/pkg/auth/login.go
+++ b/pkg/auth/login.go
@@ -35,7 +35,7 @@ func (o KubernetesClient) Login(server, username, password, token, caAuth string
a := login.LoginOptions{
Server: server,
- CommandName: "odo",
+ CommandName: "oc",
CAFile: caAuth,
InsecureTLS: skipTLS,
Username: username,
After that, everything works properly.
Thanks.
Signed-off-by: Parthvi <parthvi.vala@gmail.com> Co-authored-by: Armel Soro <asoro@redhat.com>
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
Unrelated issue with interactive E2E tests - let's keep an eye on this to see if it happens again. /override OpenShift-Integration-tests/OpenShift-Integration-tests |
@rm3l: Overrode contexts on behalf of rm3l: OpenShift-Integration-tests/OpenShift-Integration-tests In response to this:
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. |
Ports conflict issue on Windows - already reported in #6939 /override windows-integration-test/Windows-test |
@rm3l: Overrode contexts on behalf of rm3l: windows-integration-test/Windows-test In response to this:
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. |
What type of PR is this:
/kind bug
What does this PR do / why we need it:
Which issue(s) this PR fixes:
Fixes #6898
PR acceptance criteria:
Unit test
Integration test
Documentation
How to test changes / Special notes to the reviewer:
This fix is specific to OpenShift. Even though I have not been able to test this fix due to the lack of an OpenShift cluster, I believe it should still work.