You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2022/05/26 07:49:15 UTC

[GitHub] [flink-kubernetes-operator] morhidi opened a new pull request, #244: [FLINK-27520] Use admission-controller-framework in Webhook

morhidi opened a new pull request, #244:
URL: https://github.com/apache/flink-kubernetes-operator/pull/244

   The JOSDK team has released their [admission-controller-framework](https://github.com/java-operator-sdk/admission-controller-framework). We can now use it as a dependency, instead of borrowing the source code.


-- 
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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink-kubernetes-operator] gyfora commented on a diff in pull request #244: [FLINK-27520] Use admission-controller-framework in Webhook

Posted by GitBox <gi...@apache.org>.
gyfora commented on code in PR #244:
URL: https://github.com/apache/flink-kubernetes-operator/pull/244#discussion_r884251881


##########
flink-kubernetes-webhook/pom.xml:
##########
@@ -36,6 +36,19 @@ under the License.
             <groupId>org.apache.flink</groupId>
             <artifactId>flink-kubernetes-operator</artifactId>
             <version>${project.version}</version>
+            <scope>provided</scope>
+        </dependency>
+
+        <dependency>
+            <groupId>io.javaoperatorsdk</groupId>
+            <artifactId>operator-framework-framework-core</artifactId>
+            <version>${operator.sdk.admission-controller.version}</version>
+            <exclusions>
+                <exclusion>
+                    <groupId>*</groupId>

Review Comment:
   I think the slightly cleaner approach here would be to remove the exclude all and explicitly include artifacts in the shade plugin:
   
   ```
   <configuration>
   	<artifactSet>
   		<includes>
   			<include>io.javaoperatorsdk:operator-framework-framework-core</include>
   		</includes>
   	</artifactSet>
   	<transformers combine.children="append">
               <transformer
                       implementation="org.apache.maven.plugins.shade.resource.ManifestResourceTransformer">
                   <mainClass>org.apache.flink.kubernetes.operator.admission.FlinkOperatorWebhook</mainClass>
               </transformer>
           </transformers>
   </configuration>
   ``` 
   
   This will be more flexible in the future when we need to add 1-2 extra packages here



-- 
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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink-kubernetes-operator] morhidi commented on a diff in pull request #244: [FLINK-27520] Use admission-controller-framework in Webhook

Posted by GitBox <gi...@apache.org>.
morhidi commented on code in PR #244:
URL: https://github.com/apache/flink-kubernetes-operator/pull/244#discussion_r883354595


##########
flink-kubernetes-webhook/pom.xml:
##########
@@ -36,6 +36,19 @@ under the License.
             <groupId>org.apache.flink</groupId>
             <artifactId>flink-kubernetes-operator</artifactId>
             <version>${project.version}</version>
+            <scope>provided</scope>
+        </dependency>
+
+        <dependency>
+            <groupId>io.javaoperatorsdk</groupId>
+            <artifactId>operator-framework-framework-core</artifactId>
+            <version>${operator.sdk.admission-controller.version}</version>
+            <exclusions>
+                <exclusion>
+                    <groupId>*</groupId>

Review Comment:
   The admission-controller-framework is depending on the `io.fabric8:openshift-client` that pulled in an older `io.fabric8:kubernetes-client`.  It had a collision with the newer `kubernetes-client` version that we put on the class path with the the operator-shaded jar.



-- 
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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink-kubernetes-operator] morhidi commented on a diff in pull request #244: [FLINK-27520] Use admission-controller-framework in Webhook

Posted by GitBox <gi...@apache.org>.
morhidi commented on code in PR #244:
URL: https://github.com/apache/flink-kubernetes-operator/pull/244#discussion_r884434216


##########
flink-kubernetes-webhook/pom.xml:
##########
@@ -36,6 +36,19 @@ under the License.
             <groupId>org.apache.flink</groupId>
             <artifactId>flink-kubernetes-operator</artifactId>
             <version>${project.version}</version>
+            <scope>provided</scope>
+        </dependency>
+
+        <dependency>
+            <groupId>io.javaoperatorsdk</groupId>
+            <artifactId>operator-framework-framework-core</artifactId>
+            <version>${operator.sdk.admission-controller.version}</version>
+            <exclusions>
+                <exclusion>
+                    <groupId>*</groupId>

Review Comment:
   Thanks @gyfora for the suggestion, I like it better myself. Changed and pushed it. 



-- 
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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink-kubernetes-operator] morhidi commented on a diff in pull request #244: [FLINK-27520] Use admission-controller-framework in Webhook

Posted by GitBox <gi...@apache.org>.
morhidi commented on code in PR #244:
URL: https://github.com/apache/flink-kubernetes-operator/pull/244#discussion_r883354595


##########
flink-kubernetes-webhook/pom.xml:
##########
@@ -36,6 +36,19 @@ under the License.
             <groupId>org.apache.flink</groupId>
             <artifactId>flink-kubernetes-operator</artifactId>
             <version>${project.version}</version>
+            <scope>provided</scope>
+        </dependency>
+
+        <dependency>
+            <groupId>io.javaoperatorsdk</groupId>
+            <artifactId>operator-framework-framework-core</artifactId>
+            <version>${operator.sdk.admission-controller.version}</version>
+            <exclusions>
+                <exclusion>
+                    <groupId>*</groupId>

Review Comment:
   The admission-controller-framework is depending on an older `io.fabric8:kubernetes-client`.  It had some collisions with the newer version that we put on the class-path with the the operator-shaded jar. Since this code worked in the operator before, it seemed safe to remove the dependencies and use them from the operator class-path. Hopefully the validation framework will merge with the operator sdk and share the dependencies.



-- 
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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink-kubernetes-operator] morhidi commented on pull request #244: [FLINK-27520] Use admission-controller-framework in Webhook

Posted by GitBox <gi...@apache.org>.
morhidi commented on PR #244:
URL: https://github.com/apache/flink-kubernetes-operator/pull/244#issuecomment-1138283559

   cc @Aitozi @wangyang0918 


-- 
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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink-kubernetes-operator] morhidi commented on a diff in pull request #244: [FLINK-27520] Use admission-controller-framework in Webhook

Posted by GitBox <gi...@apache.org>.
morhidi commented on code in PR #244:
URL: https://github.com/apache/flink-kubernetes-operator/pull/244#discussion_r884442238


##########
flink-kubernetes-webhook/pom.xml:
##########
@@ -36,6 +36,19 @@ under the License.
             <groupId>org.apache.flink</groupId>
             <artifactId>flink-kubernetes-operator</artifactId>
             <version>${project.version}</version>
+            <scope>provided</scope>
+        </dependency>
+
+        <dependency>
+            <groupId>io.javaoperatorsdk</groupId>
+            <artifactId>operator-framework-framework-core</artifactId>
+            <version>${operator.sdk.admission-controller.version}</version>
+            <exclusions>
+                <exclusion>
+                    <groupId>*</groupId>

Review Comment:
   Had to keep the excludes in the dependency for the unit tests.



-- 
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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink-kubernetes-operator] gyfora merged pull request #244: [FLINK-27520] Use admission-controller-framework in Webhook

Posted by GitBox <gi...@apache.org>.
gyfora merged PR #244:
URL: https://github.com/apache/flink-kubernetes-operator/pull/244


-- 
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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink-kubernetes-operator] Aitozi commented on a diff in pull request #244: [FLINK-27520] Use admission-controller-framework in Webhook

Posted by GitBox <gi...@apache.org>.
Aitozi commented on code in PR #244:
URL: https://github.com/apache/flink-kubernetes-operator/pull/244#discussion_r882766816


##########
flink-kubernetes-webhook/pom.xml:
##########
@@ -36,6 +36,19 @@ under the License.
             <groupId>org.apache.flink</groupId>
             <artifactId>flink-kubernetes-operator</artifactId>
             <version>${project.version}</version>
+            <scope>provided</scope>
+        </dependency>
+
+        <dependency>
+            <groupId>io.javaoperatorsdk</groupId>
+            <artifactId>operator-framework-framework-core</artifactId>
+            <version>${operator.sdk.admission-controller.version}</version>
+            <exclusions>
+                <exclusion>
+                    <groupId>*</groupId>

Review Comment:
   Hi @morhidi just want to ask is this means to exclude all the transitive dependency ?



-- 
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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org