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

OLS-1232 - Adding consoleURL to results of get credentials #1617

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

JoaoFula
Copy link

and updating readme

Before you complete this pull request ...

Look for any open pull requests in the repository with the title "e2e-tests update" and
see if there are recent e2e-tests updates that will be applicable to your change.

@flacatus
Copy link
Contributor

/ok-to-test

@flacatus
Copy link
Contributor

/lgtm

@@ -76,3 +78,6 @@ spec:
API_SERVER_URL=$("${OC[@]}" get cti "$CLUSTER_NAME" -o=jsonpath='{.status.apiServerURL}')
echo "API Server URL: $API_SERVER_URL"
echo -n "$API_SERVER_URL" > "$(step.results.apiServerURL.path)"
CONSOLE_URL=https://$("${OC[@]}" get route console -n openshift-console -o go-template --template="{{.spec.host}}")
Copy link
Contributor

Choose a reason for hiding this comment

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

I tested this from a sample pipeline and ran into the following error which is to be expected:

step-get-kubeconfig
Found kubeconfig secret: cluster-ghqk7-admin-kubeconfig
Wrote kubeconfig to /credentials/cluster-ghqk7-kubeconfig
Found admin password secret: cluster-ghqk7-admin-password
Retrieved username
Wrote password to /credentials/cluster-ghqk7-password
API Server URL: https://ae2301db3888646799a6da0a74f33c56-7a5e7a3f524339df.elb.us-east-1.amazonaws.com:6443
Error from server (Forbidden): routes.route.openshift.io "console" is forbidden: User "system:serviceaccount:amisstea-25da3-eaas:namespace-manager" cannot get resource "routes" in API group "route.openshift.io" in the namespace "openshift-console"

This is trying to gather the console URL of the management cluster where the control plane is running. The desired route must instead be retrieved from the the ephemeral cluster using the credentials gathered above.

Copy link
Author

Choose a reason for hiding this comment

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

oh. Could this be avoided by simply exporting KUBECONFIG as the previously exported kubeconfig?
This should set the oc call to the ephemeral cluster right?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, that's generally what needs to happen.

I also think we shouldn't reuse ${OC[@]} for this call since that may include flags only intended for communicating to the management cluster (e.g. --insecure-skip-tls-verify).

Copy link
Author

Choose a reason for hiding this comment

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

changed and changed. Could I ask you to test it again please?

Copy link
Author

Choose a reason for hiding this comment

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

Will need to drop now and will come back tomorrow. If it doesn't work, could you please explain how I can test it so that I can solve it tomorrow first thing? Thanks

Copy link
Contributor

Choose a reason for hiding this comment

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

It worked 😄

To test this I forked the minimal sample pipeline from the docs and modified this StepAction's git reference to point to your changes. Then I configured an IntegrationTestScenario for a sample Konflux component and ran a build to indirectly trigger it.

@amisstea
Copy link
Contributor

LGTM

@mmorhun / @taylormadore we require your assistance to trigger the CI. Please review and provide approval as well, if possible.

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.

3 participants