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

Provision git configuration into each container #14402

Merged
merged 1 commit into from
Sep 4, 2019
Merged

Provision git configuration into each container #14402

merged 1 commit into from
Sep 4, 2019

Conversation

vzhukovs
Copy link
Contributor

@vzhukovs vzhukovs commented Sep 2, 2019

What does this PR do?

This changes proposal adds ability to provision git configuration into each container at container startup. Configuration consists of user name and user email needed for git to sign user commits.

Linked PR on the Theia side, which provide preference editor to configure the values operated in this PR: eclipse-che/che-theia#424

Demo for this PR: https://www.youtube.com/watch?v=Q_XsUWR_mJY&feature=youtu.be

Signed-off-by: Vlad Zhukovskyi vzhukovs@redhat.com

What issues does this PR fix or reference?

#13874

Release Notes

Provision git configuration into each container

Docs PR

@che-bot che-bot added kind/enhancement A feature request - must adhere to the feature request template. status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. labels Sep 2, 2019
@che-bot
Copy link
Contributor

che-bot commented Sep 2, 2019

E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has failed:

@che-bot
Copy link
Contributor

che-bot commented Sep 2, 2019

E2E tests of Eclipse Che Multiuser on OCP has failed:

@che-bot
Copy link
Contributor

che-bot commented Sep 2, 2019

E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has been successful:

  • build details
  • "che-server" docker image: maxura/che-server:14402

@che-bot
Copy link
Contributor

che-bot commented Sep 2, 2019

E2E tests of Eclipse Che Multiuser on OCP has failed:

String userId = EnvironmentContext.getCurrent().getSubject().getUserId();
Map<String, String> preferencesMap = preferenceManager.find(userId, keyFilter);

return ofNullable(preferencesMap.get(PREFERENCES_KEY_FILTER));
Copy link
Contributor

@gorkem gorkem Sep 2, 2019

Choose a reason for hiding this comment

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

This function is a bit odd for me. It is a private function that is used with PREFERENCES_KEY_FILTER as keyFilter only. And in this line it refers to PREFERENCES_KEY_FILTER directly. I feel that either it should not refer to PREFERENCES_KEY_FILTER or not have a keyFilter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, you're right! Thanks for the pointing. keyFilter argument actually has the same value as PREFERENCES_KEY_FILTER has. Will replace constant value with argument value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also will update name of the function, getPreferenceValue is more preferred name for it.

@che-bot
Copy link
Contributor

che-bot commented Sep 2, 2019

E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has been successful:

  • build details
  • "che-server" docker image: maxura/che-server:14402

@che-bot
Copy link
Contributor

che-bot commented Sep 2, 2019

E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has been successful:

  • build details
  • "che-server" docker image: maxura/che-server:14402

@che-bot
Copy link
Contributor

che-bot commented Sep 2, 2019

E2E tests of Eclipse Che Multiuser on OCP has failed:

Copy link
Contributor

@amisevsk amisevsk left a comment

Choose a reason for hiding this comment

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

LGTM

import static java.util.Collections.singletonMap;
import static java.util.Optional.empty;
import static java.util.Optional.of;
import static java.util.Optional.ofNullable;
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not a fan of static imports of methods such as these, since names like of are not descriptive at all. Up to you though, I know we do it all over the codebase currently.

Signed-off-by: Vlad Zhukovskyi <vzhukovs@redhat.com>
@che-bot
Copy link
Contributor

che-bot commented Sep 3, 2019

E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has been successful:

  • build details
  • "che-server" docker image: maxura/che-server:14402

@che-bot
Copy link
Contributor

che-bot commented Sep 3, 2019

E2E tests of Eclipse Che Multiuser on OCP has failed:

@SkorikSergey
Copy link
Contributor

Selenium tests execution on Eclipse Che Multiuser on OCP (https://ci.codenvycorp.com/job/che-pullrequests-test-ocp/2174//Selenium_20tests_20report/) doesn't show any regression against this Pull request.

@vzhukovs vzhukovs merged commit 25de692 into master Sep 4, 2019
@vzhukovs vzhukovs deleted the che#13874 branch September 4, 2019 08:37
@che-bot che-bot removed the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Sep 4, 2019
Copy link
Contributor

@amisevsk amisevsk left a comment

Choose a reason for hiding this comment

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

@vzhukovskii I was testing these changes on OpenShift, and it looks like the provisioning isn't included in OpenShiftEnvironmentProvisioner.

@vzhukovs
Copy link
Contributor Author

vzhukovs commented Sep 5, 2019

@amisevsk double checked everything, and you're right. Registration in OpenShiftEnvironmentProvisioner is missing. I'll add registration in #14442 and recheck, how it goes on OpenShift.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement A feature request - must adhere to the feature request template.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants