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

Template test is not reentrant #7078

Merged
merged 1 commit into from Feb 8, 2016

Conversation

smarterclayton
Copy link
Contributor

[test]

@openshift-bot
Copy link
Contributor

Evaluated for origin test up to 5710618

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/881/)

@@ -76,6 +79,9 @@ echo "template data precision: ok"

os::cmd::expect_success 'oc create -f examples/sample-app/application-template-dockerbuild.json -n openshift'
os::cmd::expect_success 'oc policy add-role-to-user admin test-user'
new="$(mktemp -d)/tempconfig"
os::cmd::expect_success "oc config view --raw > $new"
export KUBECONFIG=$new
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like this is done to avoid trashing the shared kubeconfig file? I'd rather give each subscript its own copy in the test-cmd.sh script.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, that doesn't fix the reentrancy problem. It ruins your local script. So this is correct as is.

Copy link
Contributor

Choose a reason for hiding this comment

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

how does copying the KUBECONFIG and invoking the tmp project creation and sub-script with KUBECONFIG=... break anything?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because the point of this change is to not require you to run hack/test-cmd.sh to test this. You just run test/cmd/templates.

@deads2k
Copy link
Contributor

deads2k commented Feb 8, 2016

The deletes lgtm, but I'd like to move the kubeconfig copy responsibility to the parent script.

@smarterclayton
Copy link
Contributor Author

I'll do a follow up with a cleaner approach to both top and bottom level specialization of the user context.

@smarterclayton
Copy link
Contributor Author

[merge]

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/881/) (Image: devenv-rhel7_3358)

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to 5710618

openshift-bot pushed a commit that referenced this pull request Feb 8, 2016
@openshift-bot openshift-bot merged commit 2a0f0f3 into openshift:master Feb 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants