nodece commented on code in PR #17148:
URL: https://github.com/apache/pulsar/pull/17148#discussion_r948875371


##########
tests/docker-images/latest-version-image/pom.xml:
##########
@@ -101,35 +108,31 @@
             </executions>
           </plugin>
           <plugin>
-            <groupId>com.spotify</groupId>
-            <artifactId>dockerfile-maven-plugin</artifactId>
-            <version>${dockerfile-maven.version}</version>
+            <groupId>io.fabric8</groupId>
+            <artifactId>docker-maven-plugin</artifactId>
             <executions>
               <execution>
                 <id>default</id>
                 <phase>package</phase>
                 <goals>
                   <goal>build</goal>
                 </goals>
-              </execution>
-              <execution>
-                <id>add-latest-tag</id>
-                <phase>package</phase>
-                <goals>
-                  <goal>tag</goal>
-                </goals>
                 <configuration>
-                  
<repository>${docker.organization}/pulsar-test-latest-version</repository>
-                  <tag>latest</tag>
+                  <images>
+                    <image>
+                      
<name>${docker.organization}/pulsar-test-latest-version</name>
+                      <build>
+                        <contextDir>${project.basedir}</contextDir>

Review Comment:
   Add:
   ```xml
   <imagePullPolicy>Never</imagePullPolicy>
   <noCache>true</noCache>
   ```



##########
tests/docker-images/latest-version-image/pom.xml:
##########
@@ -101,35 +108,31 @@
             </executions>
           </plugin>
           <plugin>
-            <groupId>com.spotify</groupId>
-            <artifactId>dockerfile-maven-plugin</artifactId>
-            <version>${dockerfile-maven.version}</version>
+            <groupId>io.fabric8</groupId>
+            <artifactId>docker-maven-plugin</artifactId>
             <executions>
               <execution>
                 <id>default</id>
                 <phase>package</phase>
                 <goals>
                   <goal>build</goal>

Review Comment:
   Add the `<goal>tag</goal>`



##########
docker/pulsar-all/pom.xml:
##########
@@ -119,57 +130,71 @@
             </executions>
           </plugin>
           <plugin>
-            <groupId>com.spotify</groupId>
-            <artifactId>dockerfile-maven-plugin</artifactId>
-            <version>${dockerfile-maven.version}</version>
+            <groupId>io.fabric8</groupId>
+            <artifactId>docker-maven-plugin</artifactId>

Review Comment:
   Add the following config:
   ```xml
   <configuration>
       <imagePullPolicy>Never</imagePullPolicy>
   </configuration>
   ```



##########
docker/pulsar/pom.xml:
##########
@@ -103,57 +107,71 @@
             </executions>
           </plugin>
           <plugin>
-            <groupId>com.spotify</groupId>
-            <artifactId>dockerfile-maven-plugin</artifactId>
-            <version>${dockerfile-maven.version}</version>
+            <groupId>io.fabric8</groupId>
+            <artifactId>docker-maven-plugin</artifactId>
             <executions>
               <execution>
                 <id>default</id>
+                <phase>package</phase>
                 <goals>
                   <goal>build</goal>
                 </goals>
-              </execution>
-              <execution>
-                <id>add-no-repo-and-version</id>
-                <goals>
-                  <goal>tag</goal>
-                </goals>
                 <configuration>
-                  <repository>pulsar</repository>
-                  <tag>${project.version}</tag>
+                  <images>
+                    <image>
+                      <name>${docker.organization}/pulsar</name>
+                      <build>
+                        <contextDir>${project.basedir}</contextDir>
+                        <tags>
+                          <tag>latest</tag>
+                          <tag>${project.version}</tag>
+                        </tags>
+                      </build>
+                    </image>
+                  </images>
                 </configuration>
               </execution>
               <execution>
-                <id>add-no-repo-and-latest</id>
+                <id>push-latest</id>

Review Comment:
   By the way, I notice that there are redundant configurations here, and I 
think we can simplify them.
   
   ```xml
   <executions>
         <execution>
           <id>default</id>
           <phase>package</phase>
           <goals>
             <goal>build</goal>
             <goal>push</goal>
           </goals>
           <configuration>
             <images>
               <image>
                 <name>${docker.organization}/pulsar</name>
                 <build>
                   <contextDir>${project.basedir}</contextDir>
                   <tags>
                     <tag>latest</tag>
                     <tag>${project.version}</tag>
                   </tags>
                 </build>
               </image>
             </images>
           </configuration>
         </execution>
   </executions>
   ```
   
   Then maybe we also need to update the 
https://github.com/apache/pulsar/blob/master/docker/publish.sh#L65
   
   So like using `${docker.organization}/pulsar:latest` instead of 
`pulsar:latest`.
   



##########
tests/docker-images/java-test-image/pom.xml:
##########
@@ -135,39 +139,31 @@
             </executions>
           </plugin>
           <plugin>
-            <groupId>com.spotify</groupId>
-            <artifactId>dockerfile-maven-plugin</artifactId>
-            <version>${dockerfile-maven.version}</version>
+            <groupId>io.fabric8</groupId>
+            <artifactId>docker-maven-plugin</artifactId>
             <executions>
               <execution>
                 <id>default</id>
                 <phase>package</phase>
                 <goals>
                   <goal>build</goal>

Review Comment:
   Add the `<goal>tag</goal>`.



##########
docker/pulsar/pom.xml:
##########
@@ -103,57 +107,71 @@
             </executions>
           </plugin>
           <plugin>
-            <groupId>com.spotify</groupId>
-            <artifactId>dockerfile-maven-plugin</artifactId>
-            <version>${dockerfile-maven.version}</version>
+            <groupId>io.fabric8</groupId>
+            <artifactId>docker-maven-plugin</artifactId>

Review Comment:
   Add the following config:
   ```xml
   <configuration>
       <imagePullPolicy>Never</imagePullPolicy>
   </configuration>
   ```



##########
tests/docker-images/java-test-image/pom.xml:
##########
@@ -135,39 +139,31 @@
             </executions>
           </plugin>
           <plugin>
-            <groupId>com.spotify</groupId>
-            <artifactId>dockerfile-maven-plugin</artifactId>
-            <version>${dockerfile-maven.version}</version>
+            <groupId>io.fabric8</groupId>
+            <artifactId>docker-maven-plugin</artifactId>
             <executions>
               <execution>
                 <id>default</id>
                 <phase>package</phase>
                 <goals>
                   <goal>build</goal>
                 </goals>
-              </execution>
-              <execution>
-                <id>add-latest-tag</id>
-                <phase>package</phase>
-                <goals>
-                  <goal>tag</goal>
-                </goals>
                 <configuration>
-                  
<repository>${docker.organization}/java-test-image</repository>
-                  <tag>latest</tag>
+                  <images>
+                    <image>
+                      <name>${docker.organization}/java-test-image</name>
+                      <build>
+                        <contextDir>${project.basedir}</contextDir>

Review Comment:
   Add:
   ```xml
   <imagePullPolicy>Never</imagePullPolicy>
   <noCache>true</noCache>
   ```



##########
docker/pulsar-all/pom.xml:
##########
@@ -119,57 +130,71 @@
             </executions>
           </plugin>
           <plugin>
-            <groupId>com.spotify</groupId>
-            <artifactId>dockerfile-maven-plugin</artifactId>
-            <version>${dockerfile-maven.version}</version>
+            <groupId>io.fabric8</groupId>
+            <artifactId>docker-maven-plugin</artifactId>
             <executions>
               <execution>
                 <id>default</id>
+                <phase>package</phase>
                 <goals>
                   <goal>build</goal>
                 </goals>
-              </execution>
-              <execution>
-                <id>add-no-repo-and-version</id>
-                <goals>
-                  <goal>tag</goal>
-                </goals>
                 <configuration>
-                  <repository>pulsar-all</repository>
-                  <tag>${project.version}</tag>
+                  <images>

Review Comment:
   By the way, I notice that there are redundant configurations here, and I 
think we can simplify them.
   
   ```xml
   <executions>
         <execution>
           <id>default</id>
           <phase>package</phase>
           <goals>
             <goal>build</goal>
             <goal>push</goal>
           </goals>
           <configuration>
             <images>
               <image>
                 <name>${docker.organization}/pulsar</name>
                 <build>
                   <contextDir>${project.basedir}</contextDir>
                   <tags>
                     <tag>latest</tag>
                     <tag>${project.version}</tag>
                   </tags>
                 </build>
               </image>
             </images>
           </configuration>
         </execution>
   </executions>
   ```
   
   Then maybe we also need to update the 
https://github.com/apache/pulsar/blob/master/docker/publish.sh#L66
   
   So like using `${docker.organization}/pulsar-all:latest` instead of 
`pulsar-all:latest`.
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to