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.

Sign up for GitHub

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

Add support to remote kafka plugin #316

Closed
wants to merge 1 commit into from
Closed

Add support to remote kafka plugin #316

Show file tree
Hide file tree
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
7 changes: 6 additions & 1 deletion plugin/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
<guava.version>20.0</guava.version> <!-- version compatible with openstack4j -->
<jsr305.version>1.3.9</jsr305.version>
<openstack4j.version>3.8</openstack4j.version>
<okhttp.version>3.9.1</okhttp.version>
<okhttp.version>3.12.0</okhttp.version>
<configuration-as-code.version>1.38</configuration-as-code.version>
</properties>

Expand Down Expand Up @@ -201,6 +201,11 @@
<version>${configuration-as-code.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>io.jenkins.plugins.remoting-kafka</groupId>
<artifactId>remoting-kafka</artifactId>
<version>2.0.1</version>
</dependency>
</dependencies>

<build>
Expand Down
2 changes: 2 additions & 0 deletions plugin/src/main/java/jenkins/plugins/openstack/compute/JCloudsCloud.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,8 @@ private Object readResolve() {
LauncherFactory lf = null;
if ("JNLP".equals(slaveOptions.slaveType)) {
lf = LauncherFactory.JNLP.JNLP;
} else if ("KAFKA".equals(slaveOptions.slaveType)) {
Copy link
Member

Choose a reason for hiding this comment

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

There is no need to add these into readResolve methods. They are only used to migrate old config to new one. And since kafka is just being added, it does not exist in user configs.

lf = LauncherFactory.KAFKA.KAFKA;
} else if (!"JNLP".equals(slaveOptions.slaveType) && slaveOptions.credentialsId != null) {
// user configured credentials and clearly rely on SSH launcher that used to be the default so bring it back
lf = new LauncherFactory.SSH(slaveOptions.credentialsId);
Expand Down
13 changes: 9 additions & 4 deletions plugin/src/main/java/jenkins/plugins/openstack/compute/JCloudsSlave.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,10 +159,15 @@ protected Object readResolve() {
.jvmOptions(Util.fixEmpty(jvmOptions))
;

LauncherFactory lf = "SSH".equals(slaveType)
? new LauncherFactory.SSH(credentialsId)
: LauncherFactory.JNLP.JNLP
;
LauncherFactory lf = null;
if ("SSH".equals(slaveType)) {
lf = new LauncherFactory.SSH(credentialsId);
} else if ("JNLP".equals(slaveType)) {
lf = LauncherFactory.JNLP.JNLP;
} else if ("KAFKA".equals(slaveType)) {
Copy link
Member

Choose a reason for hiding this comment

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

There is no need to add these into readResolve methods. They are only used to migrate old config to new one. And since kafka is just being added, it does not exist in user configs.

lf = LauncherFactory.KAFKA.KAFKA;
}

builder.launcherFactory(lf);

if (overrideRetentionTime > 0) {
Expand Down
5 changes: 5 additions & 0 deletions plugin/src/main/java/jenkins/plugins/openstack/compute/JCloudsSlaveTemplate.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,8 @@ private Object readResolve() {
lf = new LauncherFactory.SSH(credentialsId);
} else if("JNLP".equals(slaveType)) {
lf = LauncherFactory.JNLP.JNLP;
} else if("KAFKA".equals(slaveType)) {
Copy link
Member

Choose a reason for hiding this comment

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

There is no need to add these into readResolve methods. They are only used to migrate old config to new one. And since kafka is just being added, it does not exist in user configs.

lf = LauncherFactory.KAFKA.KAFKA;
}

BootSource.Image bs = imageId == null ? null : new BootSource.Image(imageId);
Expand Down Expand Up @@ -153,6 +155,9 @@ private Object readResolve() {
} else if ("SSH".equals(slaveOptions.slaveType)) {
lf = new LauncherFactory.SSH(slaveOptions.credentialsId);
slaveOptions.slaveType = slaveOptions.credentialsId = null;
} else if ("KAFKA".equals(slaveOptions.slaveType)) {
Copy link
Member

Choose a reason for hiding this comment

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

There is no need to add these into readResolve methods. They are only used to migrate old config to new one. And since kafka is just being added, it does not exist in user configs.

lf = LauncherFactory.KAFKA.KAFKA;
slaveOptions.slaveType = null;
}
if (lf != null) {
slaveOptions = slaveOptions.getBuilder().launcherFactory(lf).build();
Expand Down
11 changes: 11 additions & 0 deletions plugin/src/main/java/jenkins/plugins/openstack/compute/UserDataVariableResolver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import hudson.Util;
import hudson.util.VariableResolver;
import jenkins.slaves.JnlpAgentReceiver;
import io.jenkins.plugins.remotingkafka.KafkaSecretManager;

import javax.annotation.Nonnull;
import java.util.LinkedHashMap;
Expand Down Expand Up @@ -77,6 +78,16 @@
"Labels of the node.",
r -> r.labelString
);
stub(
"SLAVE_KAFKA_SECRET",
Copy link
Member

Choose a reason for hiding this comment

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

Please refrain from using the word slave and prefer agent. https://www.jenkins.io/blog/2020/06/18/terminology-update/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just want to be compliant with SLAVE_JNLP_SECRET variable.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, we would have to rename that one as well.

"Kafka secret.",
r -> Util.fixNull(KafkaSecretManager.getConnectionSecret(r.serverName))
);
stub(
"SLAVE_NAME",
Copy link
Member

Choose a reason for hiding this comment

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

Please refrain from using the word slave and prefer agent. https://www.jenkins.io/blog/2020/06/18/terminology-update/

"SLAVE_NAME.",
r -> Util.fixNull(r.serverName)
);
}
private static void stub(@Nonnull String name, @Nonnull String doc, @Nonnull ValueCalculator vc) {
STUB.put(name, new Entry(name, doc, vc));
Expand Down
48 changes: 48 additions & 0 deletions plugin/src/main/java/jenkins/plugins/openstack/compute/slaveopts/LauncherFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@
import java.util.logging.Level;
import java.util.logging.Logger;

import io.jenkins.plugins.remotingkafka.KafkaComputerLauncher;
/**
* Node launcher factory.
*
Expand Down Expand Up @@ -272,6 +273,53 @@ public LauncherFactory newInstance(StaplerRequest req, @Nonnull JSONObject formD
}
}

/**
* Wait for KAFKA connection to be made.
*/
public static final class KAFKA extends LauncherFactory {
private static final long serialVersionUID = -1112849796889317241L;

public static final LauncherFactory KAFKA = new KAFKA();

@DataBoundConstructor // Needed for JCasC
public KAFKA() {}

@Override
public ComputerLauncher createLauncher(@Nonnull JCloudsSlave slave) throws IOException {
Jenkins.get().addNode(slave);
return new KafkaComputerLauncher();
}

@Override
public @CheckForNull String isWaitingFor(@Nonnull JCloudsSlave slave) {
// The address might not be visible at all so let's just wait for connection.
return slave.getChannel() != null ? null : "KAFKA connection was not established yet";
}

@Override
public int hashCode() {
return 31;
}

@Override
public boolean equals(Object obj) {
return obj != null && getClass() == obj.getClass();
}

private Object readResolve() {
return KAFKA; // Let's avoid creating instances where we can
}

@Extension
@Symbol("kafka")
public static final class Desc extends Descriptor<LauncherFactory> {
@Override
public LauncherFactory newInstance(StaplerRequest req, @Nonnull JSONObject formData) {
return KAFKA; // Let's avoid creating instances where we can
}
}
}

/**
* No slave type specified. This exists only as a field in UI dropdown to be read by stapler and converted to plain old null.
*/
Expand Down
9 changes: 9 additions & 0 deletions .../resources/jenkins/plugins/openstack/compute/slaveopts/LauncherFactory/KAFKA/config.jelly
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<?jelly escape-by-default='true'?>
<j:jelly xmlns:j="jelly:core" xmlns:f="/lib/form">
<f:section title="KAFKA remote agent launch">
<f:entry>
<p>Jenkins will wait for the Kafka connection to be initiated from the slave itself. Note this plugin does not initiate
Copy link
Member

@olivergondza olivergondza Apr 23, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Please refrain from using the word slave and prefer agent. https://www.jenkins.io/blog/2020/06/18/terminology-update/

that. See help for <i>User Data</i> field for more information on the available options.</p>
Copy link
Member

Choose a reason for hiding this comment

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

We might need to massage this a bit, by ideally moving the details here. For now, JNLP was the only inbound connection supported so the user-data config mentioned that only. Kafka seems to require specific launching instructions, that needs to be documented here (or at least link to plugin documentation). Also, there will be no way for the other plugin to contribute documentation for User Data, so this might be the only place to document it.

</f:entry>
</f:section>
</j:jelly>