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

feat(openshift): Run and Test actions #4730

Merged
merged 1 commit into from
Jul 3, 2023
Merged

Conversation

Walther
Copy link
Contributor

@Walther Walther commented Jun 30, 2023

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #4626
Fixes #4627

Special notes for your reviewer:

@Walther Walther requested a review from a team June 30, 2023 10:38
@@ -233,6 +233,11 @@ export async function getSystemNamespace(
api?: KubeApi
): Promise<string> {
const namespace = { name: provider.config.gardenSystemNamespace }
// HACK: in OpenShift, we work in only one namespace, the one assigned to the developer
if (!namespace.name) {
log.warn("No system namespace found, using the current namespace. If you are using OpenShift, ignore this warning.")
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason to print the message in the first place for openshift users?

Copy link
Contributor Author

@Walther Walther Jul 3, 2023

Choose a reason for hiding this comment

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

This is within the generic kubernetes code, and we don't necessarily know here if we are running OpenShift or not. Something to fix later when we have the OpenShfit provider in a better shape and can propagate the information around

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the ctx passed here has that data though

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently, we don't have a proper OpenShift provider context - it is using the KubernetesProvider and its context under the hood. Definitely something we need to improve later!

Copy link
Contributor

@shumailxyz shumailxyz left a comment

Choose a reason for hiding this comment

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

LGTM, Thank you 💯

@Walther Walther merged commit 46ec532 into main Jul 3, 2023
@Walther Walther deleted the local-openshift-run-and-test branch July 3, 2023 11:02
ShankyJS pushed a commit that referenced this pull request Jul 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants