You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by "dmvolod (via GitHub)" <gi...@apache.org> on 2023/03/09 19:56:05 UTC

[GitHub] [camel-karavan] dmvolod opened a new pull request, #673: Operator should deploy Task and Pipelines if Tekton installed after Karavan

dmvolod opened a new pull request, #673:
URL: https://github.com/apache/camel-karavan/pull/673

   fixes #567


-- 
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: commits-unsubscribe@camel.apache.org

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


[GitHub] [camel-karavan] mgubaidullin merged pull request #673: Operator should deploy Task and Pipelines if Tekton installed after Karavan

Posted by "mgubaidullin (via GitHub)" <gi...@apache.org>.
mgubaidullin merged PR #673:
URL: https://github.com/apache/camel-karavan/pull/673


-- 
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: commits-unsubscribe@camel.apache.org

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


[GitHub] [camel-karavan] mgubaidullin commented on a diff in pull request #673: Operator should deploy Task and Pipelines if Tekton installed after Karavan

Posted by "mgubaidullin (via GitHub)" <gi...@apache.org>.
mgubaidullin commented on code in PR #673:
URL: https://github.com/apache/camel-karavan/pull/673#discussion_r1134051715


##########
karavan-operator/src/main/java/org/apache/camel/karavan/operator/KaravanReconciler.java:
##########
@@ -166,46 +166,6 @@ private void initDependentResources() {
         });
     }
 
-    public void addTektonResources() {

Review Comment:
   Where actually we create Pipelines and Tasks as we removed it 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: commits-unsubscribe@camel.apache.org

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


[GitHub] [camel-karavan] dmvolod commented on a diff in pull request #673: Operator should deploy Task and Pipelines if Tekton installed after Karavan

Posted by "dmvolod (via GitHub)" <gi...@apache.org>.
dmvolod commented on code in PR #673:
URL: https://github.com/apache/camel-karavan/pull/673#discussion_r1134057512


##########
karavan-operator/src/main/java/org/apache/camel/karavan/operator/KaravanReconciler.java:
##########
@@ -166,46 +166,6 @@ private void initDependentResources() {
         });
     }
 
-    public void addTektonResources() {

Review Comment:
   We can create them on operator startup only and once. Other options are not supported by the JOSDK.
   That's is the reason why we need to restart whole operator application if we detects that Tekton pipelines CRD's  are available after the Karavan operator starts.



-- 
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: commits-unsubscribe@camel.apache.org

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


[GitHub] [camel-karavan] dmvolod commented on a diff in pull request #673: Operator should deploy Task and Pipelines if Tekton installed after Karavan

Posted by "dmvolod (via GitHub)" <gi...@apache.org>.
dmvolod commented on code in PR #673:
URL: https://github.com/apache/camel-karavan/pull/673#discussion_r1134129865


##########
karavan-operator/src/main/java/org/apache/camel/karavan/operator/KaravanReconciler.java:
##########
@@ -166,46 +166,6 @@ private void initDependentResources() {
         });
     }
 
-    public void addTektonResources() {

Review Comment:
   Why not.
   It should be covered by tests in #682 but it's normal behavior with the Pod created from Deployment, if someone happens with it inside.



-- 
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: commits-unsubscribe@camel.apache.org

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


[GitHub] [camel-karavan] dmvolod commented on a diff in pull request #673: Operator should deploy Task and Pipelines if Tekton installed after Karavan

Posted by "dmvolod (via GitHub)" <gi...@apache.org>.
dmvolod commented on code in PR #673:
URL: https://github.com/apache/camel-karavan/pull/673#discussion_r1134057512


##########
karavan-operator/src/main/java/org/apache/camel/karavan/operator/KaravanReconciler.java:
##########
@@ -166,46 +166,6 @@ private void initDependentResources() {
         });
     }
 
-    public void addTektonResources() {

Review Comment:
   We can create them on operator startup only and once. Other options are not supported by the OJSDK.
   That's is the reason why we need to restart whole operator application if we detects that Tekton pipelines CRD's  are available after the Karavan operator starts.



-- 
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: commits-unsubscribe@camel.apache.org

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


[GitHub] [camel-karavan] mgubaidullin commented on a diff in pull request #673: Operator should deploy Task and Pipelines if Tekton installed after Karavan

Posted by "mgubaidullin (via GitHub)" <gi...@apache.org>.
mgubaidullin commented on code in PR #673:
URL: https://github.com/apache/camel-karavan/pull/673#discussion_r1134121226


##########
karavan-operator/src/main/java/org/apache/camel/karavan/operator/KaravanReconciler.java:
##########
@@ -166,46 +166,6 @@ private void initDependentResources() {
         });
     }
 
-    public void addTektonResources() {

Review Comment:
   Will Kubernetes restart operator after we stop it with `Quarkus.asyncExit();` ?
   



-- 
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: commits-unsubscribe@camel.apache.org

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


[GitHub] [camel-karavan] mgubaidullin commented on a diff in pull request #673: Operator should deploy Task and Pipelines if Tekton installed after Karavan

Posted by "mgubaidullin (via GitHub)" <gi...@apache.org>.
mgubaidullin commented on code in PR #673:
URL: https://github.com/apache/camel-karavan/pull/673#discussion_r1134132264


##########
karavan-operator/src/main/java/org/apache/camel/karavan/operator/KaravanReconciler.java:
##########
@@ -166,46 +166,6 @@ private void initDependentResources() {
         });
     }
 
-    public void addTektonResources() {

Review Comment:
   Lets try )



-- 
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: commits-unsubscribe@camel.apache.org

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


[GitHub] [camel-karavan] mgubaidullin commented on a diff in pull request #673: Operator should deploy Task and Pipelines if Tekton installed after Karavan

Posted by "mgubaidullin (via GitHub)" <gi...@apache.org>.
mgubaidullin commented on code in PR #673:
URL: https://github.com/apache/camel-karavan/pull/673#discussion_r1134047165


##########
karavan-operator/src/main/java/org/apache/camel/karavan/operator/watcher/TektonCrdEventHandler.java:
##########
@@ -16,36 +16,48 @@
  */
 package org.apache.camel.karavan.operator.watcher;
 
+import io.fabric8.kubernetes.api.model.HasMetadata;
 import io.fabric8.kubernetes.api.model.apiextensions.v1.CustomResourceDefinition;
+import io.fabric8.kubernetes.api.model.apiextensions.v1.CustomResourceDefinitionSpec;
+import io.fabric8.kubernetes.client.KubernetesClient;
 import io.fabric8.kubernetes.client.informers.ResourceEventHandler;
-import org.apache.camel.karavan.operator.KaravanReconciler;
+import io.quarkus.runtime.Quarkus;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
-public class TektonCrdEventHandler implements ResourceEventHandler<CustomResourceDefinition> {
+import java.util.Objects;
+import java.util.function.Supplier;
 
-    private static final String NAME = "pipelines.tekton.dev";
+public class TektonCrdEventHandler<T extends HasMetadata> implements ResourceEventHandler<CustomResourceDefinition> {
+    static final Logger log = LoggerFactory.getLogger(TektonCrdEventHandler.class);
 
-    private KaravanReconciler karavanReconciler;
+    private final Supplier<? extends T> obj;
 
-    public TektonCrdEventHandler(KaravanReconciler karavanReconciler) {
-        this.karavanReconciler = karavanReconciler;
+    private boolean initTektonInstalled;
+
+    public TektonCrdEventHandler(Supplier<? extends T> obj, boolean initTektonInstalled) {
+        this.obj = Objects.requireNonNull(obj);
+        this.initTektonInstalled = initTektonInstalled;
     }
 
     @Override
     public void onAdd(CustomResourceDefinition crd) {
-        if (crd.getMetadata().getName().contains(NAME)) {
-            karavanReconciler.addTektonResources();
+        if (!initTektonInstalled && isTektonCustomResource(crd.getSpec())) {
+            // This is hack while OJSDK doesn't support dynamic EventSource reload.

Review Comment:
   I don't get this point. could you please explain?



-- 
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: commits-unsubscribe@camel.apache.org

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