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

KUBECONFIG somehow reverted back to the original path in CI #3501

Closed
prietyc123 opened this issue Jul 7, 2020 · 14 comments · Fixed by #3528
Closed

KUBECONFIG somehow reverted back to the original path in CI #3501

prietyc123 opened this issue Jul 7, 2020 · 14 comments · Fixed by #3528
Labels
area/testing Issues or PRs related to testing, Quality Assurance or Quality Engineering kind/bug Categorizes issue or PR as related to a bug.

Comments

@prietyc123
Copy link
Contributor

/kind bug

What versions of software are you using?

Operating System:
supported

Output of odo version:
master

How did you run odo exactly?

Tests running on openshift repo verifies that https://deck-ci.apps.ci.l2s4.p1.openshiftapps.com/view/gcs/origin-ci-test/pr-logs/pull/openshift_release/9431/rehearse-9431-periodic-ci-openshift-odo-master-v4.3-integration-e2e-periodic-steps/1280376323939766272#1:build-log.txt%3A323

Though we are already setting up the KUBECONFIG environment var to a temporary directory which can be seen from test logs also.

Not sure whether it is getting reverted back or it is a problem from our odo side. I will send a pr to confirm this weird behaviour on CI.

Actual behavior

Though KUBECONFIG is set to a temporary file but during test run original KUBECONFIG getting fetched.

Expected behavior

KUBECONFIG should not get reverted to original one.

Any logs, error output, etc?

https://deck-ci.apps.ci.l2s4.p1.openshiftapps.com/view/gcs/origin-ci-test/pr-logs/pull/openshift_release/9431/rehearse-9431-periodic-ci-openshift-odo-master-v4.3-integration-e2e-periodic-steps/1280376323939766272#1:build-log.txt%3A323

@openshift-ci-robot openshift-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Jul 7, 2020
@amitkrout
Copy link
Contributor

@prietyc123 Branch https://github.com/openshift/odo/tree/multistage has been created. You can experiment your stuff in no time.

@prietyc123
Copy link
Contributor Author

prietyc123 commented Jul 8, 2020

Branch https://github.com/openshift/odo/tree/multistage has been created. You can experiment your stuff in no time.

I was thinking to use multistage branch to cover all the issue related to #3270 in one go and then use master branch for final pr. But Sadly we can not do that with any branch other than master. I suspect it is due to openshift infra and how they are handling the other openshift tooling. However I tried using multistage branch but failed https://deck-ci.apps.ci.l2s4.p1.openshiftapps.com/view/gcs/origin-ci-test/pr-logs/pull/openshift_release/9431/pull-ci-openshift-release-master-ci-operator-config-metadata/1280528691843043328#1:build-log.txt%3A6

Hence we have to work with the same process as we are doing. What we can do is if the changes are not breaking master and valid, merge it asap or forcefully to our repo. It will reduce the unwanted wastage of time.

@prietyc123
Copy link
Contributor Author

prietyc123 commented Jul 8, 2020

I have observed that the KUBECONFIG is getting reverted somehow to original kubeconfig before test execution itself. It is not the issue from odo side as I tried checking here before any odo command executes and found it changed https://storage.googleapis.com/origin-ci-test/pr-logs/pull/openshift_odo/3503/pull-ci-openshift-odo-master-v4.4-integration-e2e/1280480628336234496/artifacts/integration-e2e/container-logs/test.log

However still I am not clear what is reverting it back to the original kubeconfig from the temporary one . @kadel @girishramnani @mohammedzee1000 @amitkrout Can you please share thoughts on this?

@amitkrout
Copy link
Contributor

I have observed that the KUBECONFIG is getting reverted somehow to original kubeconfig before test execution itself. It is not the issue from odo side as I tried checking here before any odo command executes and found it changed https://storage.googleapis.com/origin-ci-test/pr-logs/pull/openshift_odo/3503/pull-ci-openshift-odo-master-v4.4-integration-e2e/1280480628336234496/artifacts/integration-e2e/container-logs/test.log

However still I am not clear what is reverting it back to the original kubeconfig from the temporary one . @kadel @girishramnani @mohammedzee1000 @amitkrout Can you please share thoughts on this?

i am looking into it

@kadel
Copy link
Member

kadel commented Jul 8, 2020

/kind test

@prietyc123 Please attach kind/test label into the issues that are related to test infra test suite etc...

@amitkrout
Copy link
Contributor

amitkrout commented Jul 8, 2020

The scope of the exported variable lives within the shell script instance while calling the script through make, so it does not reflect in the current shell. For example

$ cat scripts/test.sh 
+ cat scripts/test.sh
#!/bin/bash
set -x
export MYTESTPATH=/Users/amit/go/src/github.com/openshift/odo/auth/mytestpath

// PHONY target in MAKEFILE
.PHONY: configure-mytest-path
configure-mytest-path: configure-mytest-path
	. ./scripts/test.sh

$ echo $MYTESTPATH
+ echo

$ make configure-mytest-path
+ make configure-mytest-path
make: Circular configure-mytest-path <- configure-mytest-path dependency dropped.
. ./scripts/test.sh
++ export MYTESTPATH=/Users/amit/go/src/github.com/openshift/odo/auth/mytestpath
++ MYTESTPATH=/Users/amit/go/src/github.com/openshift/odo/auth/mytestpath

$ echo $MYTESTPATH
+ echo

$

This need to be fixed through MAKEFILE itself

@kadel
Copy link
Member

kadel commented Jul 8, 2020

@prietyc123
Copy link
Contributor Author

prietyc123 commented Jul 8, 2020

Me and Amit spend some time to get it done through the makefile. But we couldn't breakthrough it. Our plan was to export a KUBECONFIG globally available to the makefile, so that it captures the final kubeconfig in the current shell instance. For example we used the below .PHONY target for our testing.

.PHONY: configure-mytest-path
configure-mytest-path: export MYTESTPATH = $(KUBECONFIG)
    . ./scripts/test.sh

Each time it was throwing runtime error of *** commands commence before first target. Stop. . To fix it we did so many manipulation/tries but non of the time it succeeded.

The test script used:

$ cat scripts/test.sh
#!/bin/bash 
set -x 
export MYTESTPATH=/Users/pkumari/go/src/github.com/openshift/odo/auth/mytestpath 

Also we discussed the alternative way. But fixing it through makefile is a ideal solution IMO. Anyway let me explore it more on fixing it.

Thanks @amitkrout for the input.

@prietyc123
Copy link
Contributor Author

prietyc123 commented Jul 8, 2020

You should be able to do something like this

https://github.com/openshift/odo/blob/e367c334c2e6a15f584821c34af4659bb2658a30/Makefile#L35

Yes, we explored this option as well but didn't find any luck. The steps we followed:

//Makefile
export MYTESTPATH ?= $(KUBECONFIG)

.PHONY: configure-mytest-path
configure-mytest-path:
    . ./scripts/test.sh

Running Test script:

$ cat scripts/test.sh
#!/bin/bash 
set -x 
export MYTESTPATH=/Users/pkumari/go/src/github.com/openshift/odo/auth/mytestpath 

Executed output:

$ make configure-mytest-path
+ make configure-mytest-path
. ./scripts/test.sh
++ export MYTESTPATH=/Users/pkumari/go/src/github.com/openshift/odo/auth/mytestpath
++ MYTESTPATH=/Users/pkumari/go/src/github.com/openshift/odo/auth/mytestpath

$ echo $MYTESTPATH

Ideally it should overwrite the MYTESTPATH with /Users/pkumari/go/src/github.com/openshift/odo/auth/mytestpath but it doesn't happen in the current shell instance.

@amitkrout
Copy link
Contributor

amitkrout commented Jul 9, 2020

@prietyc123 In make target once the child process starts through the .PHONY statement, its environment is completely its own. The children will have their own environment copies and can change those, but they have no impact on the parent.

So the suggested way you are looking for can not be implemented through passing variable in make target. But it can be doable through modifying few more scripts. For example

Replace https://github.com/openshift/odo/blob/master/scripts/configure-installer-tests-cluster.sh#L17 with

ORIGINAL_KUBECONFIG=${KUBECONFIG:-"${DEFAULT_INSTALLER_ASSETS_DIR}/auth/kubeconfig"}
export KUBECONFIG=$ORIGINAL_KUBECONFIG

And at the end of the file add a KUBECONFIG clean up step for the child instance

# KUBECONFIG cleanup only if CI is set
if [ ! -f $CI ]; then
    rm -rf $KUBECONFIG
    export KUBECONFIG=$ORIGINAL_KUBECONFIG
fi

And the rest change you have tested through the pr https://github.com/openshift/odo/pull/3503/files#diff-7b5f3e630b18a516b0827419c75d2234R17-R21.

Please create a separate pr whit all these changes i suggested.

@prietyc123
Copy link
Contributor Author

@prietyc123 In make target once the child process starts through the .PHONY statement, its environment is completely its own. The children will have their own environment copies and can change those, but they have no impact on the parent.

Hmm… Then i am pretty sure we have never tested our test script as a developer in prow CI. Let me confirm it through my debug pr #3503.
Thanks @amitkrout for the input

@amitkrout
Copy link
Contributor

Hmm… Then i am pretty sure we have never tested our test script as a developer in prow CI. Let me confirm it through my debug pr #3503.

Right, the child recipes can not be copied to the parent shell. i can see it locally too

$ oc whoami
kube:admin
$ make configure-installer-tests-cluster
. ./scripts/configure-installer-tests-cluster.sh
[...]
++ oc login -u developer -p developer
[...]
++ sleep 4
++ oc version
Client Version: 4.3.0-0.nightly-2020-04-28-044647
Kubernetes Version: v1.18.3+a377312
$ oc whoami
kube:admin

@prietyc123
Copy link
Contributor Author

+ oc whoami
system:admin
++ mktemp -d
+ TMP_DIR=/tmp/tmp.XWeCeifsLG
+ cp /tmp/admin.kubeconfig /tmp/tmp.XWeCeifsLG/kubeconfig
+ chmod 640 /tmp/tmp.XWeCeifsLG/kubeconfig
+ export KUBECONFIG=/tmp/tmp.XWeCeifsLG/kubeconfig
+ KUBECONFIG=/tmp/tmp.XWeCeifsLG/kubeconfig
+ oc whoami
system:admin
+ make test-cmd-project

yup, we never tested our test script from a developer login - https://storage.googleapis.com/origin-ci-test/pr-logs/pull/openshift_odo/3503/pull-ci-openshift-odo-master-v4.3-integration-e2e/1281079863381331968/artifacts/integration-e2e/container-logs/test.log

@amitkrout
Copy link
Contributor

@kadel kadel added area/testing Issues or PRs related to testing, Quality Assurance or Quality Engineering and removed kind/test labels Jul 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/testing Issues or PRs related to testing, Quality Assurance or Quality Engineering kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants