Updating config cmds and tests for logged out senario #4975

mohammedzee1000 · 2021-08-09T13:24:33Z

Signed-off-by: Mohammed Zeeshan Ahmed mohammed.zee1000@gmail.com

What type of PR is this?
See topic
Also, identified 2 cases:

invalid kubeconfig path
Kubeconfig was not being handled.

Essentially it boils down to us not handling error from context.New()

/kind bug

What does this PR do / why we need it:
See topic

Which issue(s) this PR fixes:

Fixes #4910

PR acceptance criteria:

How to test changes / Special notes to the reviewer:

> odo create nodejs
Devfile Object Validation
 ✓  Checking devfile existence [87112ns]
 ✓  Creating a devfile component from registry: DefaultDevfileRegistry [150916ns]
Validation
 ✓  Validating if devfile name is correct [114120ns]

Please use `odo push` command to create the component with source deployed
> odo config view
ComponentName: nodejs
Configs:
- ContainerName: runtime
  Ports:
  - ExposedPort: 3000
    Name: http-3000
    Protocol: http
Memory: 1024Mi
> mv ~/.kube/config ~/.kube/configback
> ls ~/.kube
cache  configback  http-cache
> odo config view
ComponentName: nodejs
Configs:
- ContainerName: runtime
  Ports:
  - ExposedPort: 3000
    Name: http-3000
    Protocol: http
Memory: 1024Mi

> odo config set --env hello=world
 ✗  invalid KUBECONFIG provided. Please point to a valid KUBECONFIG. You do not have to be logged in 
Please ensure you have an active kubernetes context to your cluster. 
Consult your Kubernetes distribution's documentation for more details
: invalid configuration: no configuration has been provided, try setting KUBERNETES_MASTER environment variable
> odo config unset --env hello
 ✗  invalid KUBECONFIG provided. Please point to a valid KUBECONFIG. You do not have to be logged in 
Please ensure you have an active kubernetes context to your cluster. 
Consult your Kubernetes distribution's documentation for more details
: invalid configuration: no configuration has been provided, try setting KUBERNETES_MASTER environment variable
> sudo cp -avrf ~/.kube/configback ~/.kube/config
[sudo] password for mzee1000: 
'/home/mzee1000/.kube/configback' -> '/home/mzee1000/.kube/config'
> oc logout
Logged "mzee1000" out on "<redacted>"
> odo config set --env hello=world
------
 ✓  Environment variables were successfully updated

Run `odo push` command to apply changes to the cluster
> odo config view
ComponentName: nodejs
Configs:
- ContainerName: runtime
  EnvironmentVariables:
  - Name: hello
    Value: world
  Ports:
  - ExposedPort: 3000
    Name: http-3000
    Protocol: http
Memory: 1024Mi
> odo config unset --env hello
 ✓  Environment variables were successfully updated

Run `odo push` command to apply changes to the cluster
> odo config view
ComponentName: nodejs
Configs:
- ContainerName: runtime
  Ports:
  - ExposedPort: 3000
    Name: http-3000
    Protocol: http
Memory: 1024Mi

mohammedzee1000 · 2021-08-12T07:29:21Z

timeout
/test psi-unit-test-mac
/test psi-unit-test-windows

mohammedzee1000 · 2021-08-12T09:51:31Z

timeout
/test psi-unit-test-mac
/test psi-unit-test-windows

mohammedzee1000 · 2021-08-12T12:52:12Z

project list json output
/retest

mohammedzee1000 · 2021-08-16T05:41:59Z

timeout
/test psi-unit-test-windows
/test psi-unit-test-mac

dharmit

Code is adding a bunch of err1 and err2. I'm not sure why we're not just using err. Subsequent calls would either set err to nil or an error returned by the function called. What's wrong in using it?

pkg/odo/cli/catalog/list/components.go

pkg/odo/cli/config/set.go

pkg/odo/cli/config/unset.go

dharmit · 2021-08-16T09:51:38Z

@mohammedzee1000 can you please update the Changelog.md and add the steps to test the change in this PR?

mohammedzee1000 · 2021-08-16T12:27:46Z

prow error
test psi-kubernetes-integration-e2e

mohammedzee1000 · 2021-08-17T08:29:25Z

Unrelated error

• Failure [55.931 seconds]
odo devfile push command tests
/go/src/github.com/openshift/odo/tests/integration/devfile/cmd_devfile_push_test.go:18
  verify command executions
  /go/src/github.com/openshift/odo/tests/integration/devfile/cmd_devfile_push_test.go:464
    when default build and run commands are defined
    /go/src/github.com/openshift/odo/tests/integration/devfile/cmd_devfile_push_test.go:494
      should execute correct commands [It]
      /go/src/github.com/openshift/odo/tests/integration/devfile/cmd_devfile_push_test.go:495
      Unexpected error:
      
          <*errors.errorString | 0xc0003583e0>: {
              s: "cmd [ps -ef] failed with error Signal 23 (URG) caught by ps (3.3.15).\nps:ps/display.c:66: please report this bug\ncommand terminated with exit code 1\n on pod ammwlb-app-f48ddc5d7-vcmck",
          }
          cmd [ps -ef] failed with error Signal 23 (URG) caught by ps (3.3.15).
          ps:ps/display.c:66: please report this bug
          command terminated with exit code 1
           on pod ammwlb-app-f48ddc5d7-vcmck
      occurred
      /go/src/github.com/openshift/odo/tests/integration/devfile/cmd_devfile_push_test.go:514
------------------------------
•••••••••

/test v4.8-integration-e2e

Timed out

/test psi-kubernetes-integration-e2e

pkg/odo/cli/config/set.go

pkg/util/util.go

tests/integration/cmd_pref_config_test.go

pkg/odo/cli/config/set.go

dharmit · 2021-08-19T07:16:52Z

pkg/util/util.go

-		} else {
-			klog.V(4).Infof("no KUBECONFIG provided and cannot fallback to default")
-			return false
+func IsValidKubeConfigPath() error {


I'm not sure if the return value of this function should be error. Function name indicates that the response to expect from it should be either Yes or No, that is, a boolean value.

I'm more inclined at not changing the signature of the function, if possible.

dharmit · 2021-08-19T07:17:14Z

pkg/util/util.go

+func IsValidKubeConfigPath() error {
+	kubeConfigPath := os.Getenv("KUBECONFIG")
+	if kubeConfigPath != "" {
+		f1, err := os.Stat(kubeConfigPath)


Why f1 and not f? Something to do with Formula 1? 😉

pkg/util/util.go

dharmit · 2021-08-19T07:25:40Z

pkg/util/util.go

+	return nil
+}
+
+func NewInvalidKubeConfigPathError() error {


Why NewInvalidKubeConfigPathError and not InvalidKubeConfigPathError? Also, why define a function instead of defining it as an error type? For example: https://github.com/openshift/odo/blob/089b8ea26c253592cd4788ed615a698297d620a5/pkg/kclient/operators.go#L15-L17

mohammedzee1000 · 2021-08-19T10:52:08Z

time out
/test psi-unit-test-windows

mohammedzee1000 · 2021-08-20T07:09:11Z

Prow error
/retest

mohammedzee1000 · 2021-08-20T07:10:05Z

/retest

Signed-off-by: Mohammed Zeeshan Ahmed <mohammed.zee1000@gmail.com>

Co-authored-by: Dharmit Shah <shahdharmit@gmail.com>

Signed-off-by: Mohammed Zeeshan Ahmed <mohammed.zee1000@gmail.com>

Co-authored-by: Philippe Martin <contact@elol.fr>

sonarcloud · 2021-08-26T07:16:21Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
7.5% Duplication

feloy · 2021-09-01T08:46:47Z

/lgtm

mohammedzee1000 · 2021-09-01T10:25:27Z

json format project error
/retest

valaparthvi · 2021-09-01T11:22:40Z

/approve

openshift-ci · 2021-09-01T11:23:05Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: valaparthvi

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [valaparthvi]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-bot · 2021-09-01T12:58:14Z

/retest-required