Skip to content

Commit

Permalink
Fix RemoveDeploymentTriggerDecorator order
Browse files Browse the repository at this point in the history
The decorator RemoveDeploymentTriggerDecorator was not configured to be triggered before ChangeDeploymentTriggerDecorator, so sometimes the image stream tag was wrong.

Relates to failures in https://github.com/dekorateio/dekorate/actions/runs/4023231521/jobs/6913879180
  • Loading branch information
Sgitario committed Feb 2, 2023
1 parent 247fcb5 commit 15daf5a
Show file tree
Hide file tree
Showing 10 changed files with 92 additions and 119 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -79,4 +79,8 @@ public String getRepository() {
public String getGroup() {
return repository == null ? null : repository.split("/")[0];
}

public String getName() {
return repository == null ? null : repository.split("/")[1];
}
}

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@

package io.quarkus.kubernetes.deployment;

import io.dekorate.config.ConfigurationSupplier;
import io.dekorate.kubernetes.config.ImageConfiguration;
import io.dekorate.kubernetes.config.ImageConfigurationBuilder;
import io.quarkus.container.spi.ContainerImageInfoBuildItem;

/**
* Workaround for https://github.com/dekorateio/dekorate/issues/1147: Dekorate only allows providing the image info via
* suppliers, not configurators.
*/
public class ApplyImageInfoConfigurationSupplier extends ConfigurationSupplier<ImageConfiguration> {

public ApplyImageInfoConfigurationSupplier(ContainerImageInfoBuildItem image, String defaultRegistry) {
super(create(image, defaultRegistry));
}

private static ImageConfigurationBuilder create(ContainerImageInfoBuildItem image, String defaultRegistry) {
ImageConfigurationBuilder builder = new ImageConfigurationBuilder();
builder.withRegistry(image.getRegistry().orElse(defaultRegistry));
builder.withEnabled(true);
builder.withGroup(image.getGroup());
builder.withName(image.getName());
builder.withVersion(image.getTag());
return builder;
}

@Override
public boolean isExplicit() {
return true;
}
}

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@

import io.dekorate.kubernetes.annotation.ServiceType;
import io.dekorate.kubernetes.config.EnvBuilder;
import io.dekorate.kubernetes.config.ImageConfiguration;
import io.dekorate.kubernetes.config.ImageConfigurationBuilder;
import io.dekorate.kubernetes.config.Port;
import io.dekorate.kubernetes.decorator.AddAnnotationDecorator;
import io.dekorate.kubernetes.decorator.AddEnvVarDecorator;
Expand All @@ -31,7 +29,6 @@
import io.dekorate.s2i.config.S2iBuildConfig;
import io.dekorate.s2i.config.S2iBuildConfigBuilder;
import io.dekorate.s2i.decorator.AddBuilderImageStreamResourceDecorator;
import io.dekorate.s2i.decorator.AddDockerImageStreamResourceDecorator;
import io.quarkus.container.image.deployment.ContainerImageConfig;
import io.quarkus.container.image.deployment.util.ImageUtil;
import io.quarkus.container.spi.BaseImageInfoBuildItem;
Expand All @@ -48,6 +45,7 @@
import io.quarkus.deployment.pkg.PackageConfig;
import io.quarkus.deployment.pkg.builditem.OutputTargetBuildItem;
import io.quarkus.kubernetes.deployment.OpenshiftConfig.DeploymentResourceKind;
import io.quarkus.kubernetes.spi.ConfigurationSupplierBuildItem;
import io.quarkus.kubernetes.spi.ConfiguratorBuildItem;
import io.quarkus.kubernetes.spi.CustomProjectRootBuildItem;
import io.quarkus.kubernetes.spi.DecoratorBuildItem;
Expand Down Expand Up @@ -128,46 +126,42 @@ public void createLabels(OpenshiftConfig config, BuildProducer<KubernetesLabelBu
}

@BuildStep
public List<ConfiguratorBuildItem> createConfigurators(ApplicationInfoBuildItem applicationInfo,
OpenshiftConfig config, Capabilities capabilities, Optional<ContainerImageInfoBuildItem> image,
List<KubernetesPortBuildItem> ports) {

List<ConfiguratorBuildItem> result = new ArrayList<>();
public void createConfigurators(ApplicationInfoBuildItem applicationInfo,
OpenshiftConfig config, Capabilities capabilities,
Optional<ContainerImageInfoBuildItem> image,
Optional<FallbackContainerImageRegistryBuildItem> fallbackRegistry,
List<KubernetesPortBuildItem> ports,
BuildProducer<ConfiguratorBuildItem> configurators,
BuildProducer<ConfigurationSupplierBuildItem> configurationSuppliers) {

KubernetesCommonHelper.combinePorts(ports, config).values().forEach(value -> {
result.add(new ConfiguratorBuildItem(new AddPortToOpenshiftConfig(value)));
configurators.produce(new ConfiguratorBuildItem(new AddPortToOpenshiftConfig(value)));
});

result.add(new ConfiguratorBuildItem(new ApplyOpenshiftRouteConfigurator(config.route, config.expose)));
configurators.produce(new ConfiguratorBuildItem(new ApplyOpenshiftRouteConfigurator(config.route, config.expose)));

// Handle remote debug configuration for container ports
if (config.remoteDebug.enabled) {
result.add(new ConfiguratorBuildItem(new AddPortToOpenshiftConfig(config.remoteDebug.buildDebugPort())));
configurators.produce(new ConfiguratorBuildItem(new AddPortToOpenshiftConfig(config.remoteDebug.buildDebugPort())));
}

if (!capabilities.isPresent(Capability.CONTAINER_IMAGE_S2I)
&& !capabilities.isPresent("io.quarkus.openshift")
&& !capabilities.isPresent(Capability.CONTAINER_IMAGE_OPENSHIFT)) {
result.add(new ConfiguratorBuildItem(new DisableS2iConfigurator()));

image.flatMap(ContainerImageInfoBuildItem::getRegistry).ifPresent(r -> {
result.add(new ConfiguratorBuildItem(new ApplyImageRegistryConfigurator(r)));
});

image.map(ContainerImageInfoBuildItem::getGroup).ifPresent(g -> {
result.add(new ConfiguratorBuildItem(new ApplyImageGroupConfigurator(g)));
});
configurators.produce(new ConfiguratorBuildItem(new DisableS2iConfigurator()));

image.ifPresent(i -> configurationSuppliers.produce(
new ConfigurationSupplierBuildItem(
new ApplyImageInfoConfigurationSupplier(i,
fallbackRegistry.map(f -> f.getRegistry()).orElse(DOCKERIO_REGISTRY)))));
}
return result;
}

@BuildStep
public List<DecoratorBuildItem> createDecorators(ApplicationInfoBuildItem applicationInfo,
OutputTargetBuildItem outputTarget,
OpenshiftConfig config,
ContainerImageConfig containerImageConfig,
Optional<FallbackContainerImageRegistryBuildItem> fallbackRegistry,
PackageConfig packageConfig,
Optional<MetricsCapabilityBuildItem> metricsConfiguration,
Capabilities capabilities,
Expand Down Expand Up @@ -308,25 +302,11 @@ public List<DecoratorBuildItem> createDecorators(ApplicationInfoBuildItem applic
if (deploymentKind == DeploymentResourceKind.DeploymentConfig
&& !OpenshiftConfig.isOpenshiftBuildEnabled(containerImageConfig, capabilities)) {
image.ifPresent(i -> {
String registry = i.registry
.or(() -> containerImageConfig.registry)
.orElse(fallbackRegistry.map(f -> f.getRegistry()).orElse(DOCKERIO_REGISTRY));
String repositoryWithRegistry = registry + "/" + i.getRepository();
ImageConfiguration imageConfiguration = new ImageConfigurationBuilder()
.withName(name)
.withRegistry(registry)
.build();

result.add(new DecoratorBuildItem(OPENSHIFT,
new AddDockerImageStreamResourceDecorator(imageConfiguration, repositoryWithRegistry)));
String imageStreamWithTag = name + ":" + i.getTag();
result.add(new DecoratorBuildItem(OPENSHIFT, new ApplyContainerImageDecorator(name, imageStreamWithTag)));
// remove the default trigger which has a wrong version
result.add(new DecoratorBuildItem(OPENSHIFT, new RemoveDeploymentTriggerDecorator()));
// re-add the trigger with the correct version
result.add(new DecoratorBuildItem(OPENSHIFT, new ChangeDeploymentTriggerDecorator(name, imageStreamWithTag)));
new ApplyContainerImageDecorator(name, i.getName() + ":" + i.getTag())));
});
} else if (image.isPresent()) {
result.add(new DecoratorBuildItem(OPENSHIFT, new RemoveDockerImageStreamResourceDecorator(name)));
result.add(new DecoratorBuildItem(OPENSHIFT, new ApplyContainerImageDecorator(name, image.get().getImage())));
}

Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package io.quarkus.kubernetes.deployment;

import io.dekorate.kubernetes.decorator.Decorator;
import io.dekorate.kubernetes.decorator.ResourceProvidingDecorator;
import io.dekorate.s2i.decorator.AddDockerImageStreamResourceDecorator;
import io.fabric8.kubernetes.api.model.KubernetesListBuilder;
import io.fabric8.openshift.api.model.ImageStream;

/**
* Workaround for https://github.com/dekorateio/dekorate/issues/1148: Dekorate is always adding an image stream if we're
* using a docker image, so we need to remove it if we're using a Deployment resource.
*/
public class RemoveDockerImageStreamResourceDecorator extends ResourceProvidingDecorator<KubernetesListBuilder> {

private final String name;

public RemoveDockerImageStreamResourceDecorator(String name) {
this.name = name;
}

public void visit(KubernetesListBuilder list) {
list.getItems().stream()
.filter(i -> i.getMetadata().getName().equals(name) && i instanceof ImageStream)
.map(ImageStream.class::cast)
.findFirst()
.ifPresent(list::removeFromItems);
}

@Override
public Class<? extends Decorator>[] after() {
return new Class[] { AddDockerImageStreamResourceDecorator.class };
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -49,17 +49,17 @@ public void assertGeneratedResources() throws IOException {
});
assertThat(h).isInstanceOfSatisfying(DeploymentConfig.class, d -> {
Container container = d.getSpec().getTemplate().getSpec().getContainers().get(0);
assertThat(container.getImage()).isEqualTo(APP_NAME + ":1.0");
assertThat(container.getImage()).isEqualTo("app:1.0");

DeploymentTriggerImageChangeParams imageTriggerParams = d.getSpec().getTriggers().get(0).getImageChangeParams();
assertThat(imageTriggerParams.getFrom().getKind()).isEqualTo("ImageStreamTag");
assertThat(imageTriggerParams.getFrom().getName()).isEqualTo(APP_NAME + ":1.0");
assertThat(imageTriggerParams.getFrom().getName()).isEqualTo("app:1.0");
});
});

assertThat(openshiftList).filteredOn(h -> "ImageStream".equals(h.getKind())).singleElement().satisfies(h -> {
assertThat(h.getMetadata()).satisfies(m -> {
assertThat(m.getName()).isEqualTo(APP_NAME);
assertThat(m.getName()).isEqualTo("app");
});
assertThat(h).isInstanceOfSatisfying(ImageStream.class, i -> {
assertThat(i.getSpec().getDockerImageRepository()).isEqualTo("quay.io/user/app");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
//
// The purpose of this test is to assert that
// When: We run local container builds targeting Openshift using `Deployment` (instead of `DeploymentConfig`)
// Then: No BuildConfg and ImageStreams are generated and that `docker.io` is used as the default registry
// Then: No BuildConfig and ImageStreams are generated and that `docker.io` is used as the default registry
//
public class OpenshiftWithLocalDockerAndDeploymentResourceTest {

Expand Down

0 comments on commit 15daf5a

Please sign in to comment.