Skip to content
This repository has been archived by the owner on Jun 19, 2024. It is now read-only.

Keep BuildConfig configuration #1071

Closed
wants to merge 1 commit into from
Closed

Keep BuildConfig configuration #1071

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions core/src/main/java/io/fabric8/maven/core/service/openshift/OpenshiftBuildService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,8 +245,8 @@ private String updateBuildConfig(OpenShiftClient client, String buildName, Build
if (!Objects.equals(buildStrategy, spec.getStrategy()) || !Objects.equals(buildOutput, spec.getOutput())) {
client.buildConfigs().withName(buildName).edit()
.editSpec()
.withStrategy(buildStrategy)
.withOutput(buildOutput)
.withStrategy(spec.getStrategy())
.withOutput(spec.getOutput())
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 really not sure why this should fix something, as this is a no-op after all: You are re-setting the same strategy and output as it was there already. The whole idea of this method is to change the strategy (docker or binary s2i build) and the output stream if changed.

@zonArt could you please elaborate what are you trying to achieve ?

Copy link
Member

@rohanKanojia rohanKanojia Jul 25, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

@rhuss : I read the related issue filed by @zonArt , I think he wants to have his pushSecrets kept in BuildConfig to push/pull from some private registry, maybe that's why he is setting same strategy and output itself.

Agreed that this doesn't align with plugin's flow at all. But is there some way to configure plugin to include these pushSecrets into BuildConfig?

Copy link
Author

@zonArt zonArt Jul 25, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

@rohanKanojia You got it totally right, being able to keep the configuration of the buildconfig and imagestream was the needed behavior thus this PR which was maybe not intended to be merged as is but more a way to request support on how to customize those objects or to find a way to keep provided ones (maybe with a flag)

.endSpec()
.done();
log.info("Updating BuildServiceConfig %s for %s strategy", buildName, buildStrategy.getType());
Expand Down