You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by GitBox <gi...@apache.org> on 2020/10/26 14:06:22 UTC

[GitHub] [camel-k] mmelko opened a new pull request #1787: WIP: Create PodDisruptionBudget for every integration

mmelko opened a new pull request #1787:
URL: https://github.com/apache/camel-k/pull/1787


   <!-- Description -->
   Closes: #1760
   This is what I have so far. PDB is created for every integration and deleted with integration. I decided to go with new trait for the PDB but I can refactor if necessary. 
   
   
   I still need to create test for the trait and also I'm not sure if `IntegrationRunning` phase is the best here.  Any feedback appreciated.
   
   
   <!--
   Enter your extended release note in the below block. If the PR requires
   additional action from users switching to the new release, include the string
   "action required". If no release note is required, write "NONE". 
   
   You can (optionally) mark this PR with labels "kind/bug" or "kind/feature" to make sure
   the text is added to the right section of the release notes. 
   -->
   
   **Release Note**
   ```release-note
   NONE
   ```
   


----------------------------------------------------------------
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.

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



[GitHub] [camel-k] astefanutti commented on pull request #1787: Create PodDisruptionBudget for every integration

Posted by GitBox <gi...@apache.org>.
astefanutti commented on pull request #1787:
URL: https://github.com/apache/camel-k/pull/1787#issuecomment-722520633


   @lburgazzoli @nicolaferraro 👋,
   
   that PR introduces the _pod_ trait, whose purpose is to be responsible for all the aspects that directly relate to the integration pods configuration. That includes upcoming work to support:
   - Tolerations
   - Pod topology spread
   - Pod template, either inlined or by referencing a ConfigMap, addressing #1685.
   
   Technically, the _affinity_ trait should be merged into that new trait, yet we may want to keep that concern separated, at least to maintain compatibility for the 1.x lifespan.
   
   It turns out we already have traits responsible for the container and deployment aspects, but none for those of the pods. The _deployment_ trait could have been a candidate to meet that need. However it's specific to the _Deployment_ strategy, while the pod aspects can be shared across deployment strategies.
   
   WDYT?


----------------------------------------------------------------
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.

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



[GitHub] [camel-k] mmelko commented on a change in pull request #1787: WIP: Create PodDisruptionBudget for every integration

Posted by GitBox <gi...@apache.org>.
mmelko commented on a change in pull request #1787:
URL: https://github.com/apache/camel-k/pull/1787#discussion_r512219029



##########
File path: pkg/trait/pdb.go
##########
@@ -0,0 +1,103 @@
+package trait
+
+import (
+	v1 "github.com/apache/camel-k/pkg/apis/camel/v1"
+	"github.com/google/go-cmp/cmp"
+	"k8s.io/api/policy/v1beta1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/intstr"
+)
+
+type pdbTrait struct {
+	BaseTrait `property:",squash"`
+	MaxUnavailable string `property:"max-unavailable" json:"maxUnavailable,omitempty"`
+	MinAvailable   string `property:"min-available" json:"minAvailable,omitempty"`
+}
+
+func newPdbTrait() Trait {
+	return &pdbTrait{
+		BaseTrait: NewBaseTrait("pdb", 900),
+	}
+}
+
+func (t *pdbTrait) Configure(e *Environment) (bool, error) {
+	return e.IntegrationInPhase(
+		v1.IntegrationPhaseRunning,
+	), nil
+}
+
+func (t *pdbTrait) Apply(e *Environment) error {
+	pdbName := "pdb-" + e.Integration.Name + "-it"
+	var pdb *v1beta1.PodDisruptionBudget
+	var err error
+
+	if pdb, err = t.Client.PolicyV1beta1().PodDisruptionBudgets(e.DetermineNamespace()).Get(pdbName, metav1.GetOptions{}); err == nil {
+		if a := t.havePdbConfChanged(pdb); a {
+			err = t.Client.PolicyV1beta1().PodDisruptionBudgets(e.DetermineNamespace()).Delete(pdbName, &metav1.DeleteOptions{})
+			updated := t.generatePodDisruptionBudget(pdbName, e.Integration)
+			if _, err = t.Client.PolicyV1beta1().PodDisruptionBudgets(e.DetermineNamespace()).Create(updated); err == nil {
+				t.L.Info("PodDisruptionBudget updated: ", pdb.Spec)
+			}
+		}
+
+	} else {
+		t.L.Info("PodDisruptionBudget for the integration", e.Integration.Name, "not found")
+		pdb = t.generatePodDisruptionBudget(pdbName, e.Integration)
+		if _, err = t.Client.PolicyV1beta1().PodDisruptionBudgets(e.DetermineNamespace()).Create(pdb); err == nil {
+			t.L.Error(err, "error during creating pdb", err)
+		}
+	}
+	return err
+}
+
+// check if configuration for pdb has changed, if yes update values in pdbtrait
+func (t *pdbTrait) havePdbConfChanged(pdb *v1beta1.PodDisruptionBudget) bool {
+	return !cmp.Equal(t.MaxUnavailable, pdb.Spec.MaxUnavailable) || !cmp.Equal(t.MinAvailable, pdb.Spec.MinAvailable)
+}
+func (t *pdbTrait) generatePodDisruptionBudget(pdbName string, integration *v1.Integration) *v1beta1.PodDisruptionBudget {
+	controller := true
+	blockOwnerDeletion := true
+	spec := v1beta1.PodDisruptionBudgetSpec{
+		Selector: &metav1.LabelSelector{
+			MatchLabels: map[string]string{
+				"camel.apache.org/integration": integration.Name,
+			},
+		},
+	}
+	var min, max intstr.IntOrString
+
+	if t.MinAvailable == "" && t.MaxUnavailable == "" {
+		//set the default
+		max = intstr.Parse("1")
+		spec.MaxUnavailable = &max
+	} else if t.MinAvailable != "" {
+		// sets to maxUnavailable to nil because only one parameter is allowed to be set
+		min = intstr.Parse(t.MinAvailable)
+		spec.MinAvailable = &min
+		spec.MaxUnavailable = nil
+		t.MaxUnavailable = ""
+	} else {
+		//set maxAvailable
+		max = intstr.Parse(t.MaxUnavailable)
+		spec.MaxUnavailable = &max
+	}
+
+	return &v1beta1.PodDisruptionBudget{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      pdbName,
+			Namespace: integration.Namespace,
+			Labels:    integration.Labels,
+			OwnerReferences: []metav1.OwnerReference{

Review comment:
       The problem was that I it was done according the labels .. but when I looked there weren't any integration labels. I have to check again




----------------------------------------------------------------
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.

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



[GitHub] [camel-k] lburgazzoli commented on pull request #1787: Create PodDisruptionBudget for every integration

Posted by GitBox <gi...@apache.org>.
lburgazzoli commented on pull request #1787:
URL: https://github.com/apache/camel-k/pull/1787#issuecomment-722524867


   I guess this also relates to: https://github.com/apache/camel-k/issues/1657


----------------------------------------------------------------
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.

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



[GitHub] [camel-k] astefanutti commented on a change in pull request #1787: WIP: Create PodDisruptionBudget for every integration

Posted by GitBox <gi...@apache.org>.
astefanutti commented on a change in pull request #1787:
URL: https://github.com/apache/camel-k/pull/1787#discussion_r512069302



##########
File path: pkg/trait/pdb.go
##########
@@ -0,0 +1,103 @@
+package trait
+
+import (
+	v1 "github.com/apache/camel-k/pkg/apis/camel/v1"
+	"github.com/google/go-cmp/cmp"
+	"k8s.io/api/policy/v1beta1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/intstr"
+)
+
+type pdbTrait struct {
+	BaseTrait `property:",squash"`
+	MaxUnavailable string `property:"max-unavailable" json:"maxUnavailable,omitempty"`
+	MinAvailable   string `property:"min-available" json:"minAvailable,omitempty"`
+}
+
+func newPdbTrait() Trait {
+	return &pdbTrait{
+		BaseTrait: NewBaseTrait("pdb", 900),
+	}
+}
+
+func (t *pdbTrait) Configure(e *Environment) (bool, error) {
+	return e.IntegrationInPhase(
+		v1.IntegrationPhaseRunning,
+	), nil
+}
+
+func (t *pdbTrait) Apply(e *Environment) error {
+	pdbName := "pdb-" + e.Integration.Name + "-it"
+	var pdb *v1beta1.PodDisruptionBudget
+	var err error
+
+	if pdb, err = t.Client.PolicyV1beta1().PodDisruptionBudgets(e.DetermineNamespace()).Get(pdbName, metav1.GetOptions{}); err == nil {
+		if a := t.havePdbConfChanged(pdb); a {
+			err = t.Client.PolicyV1beta1().PodDisruptionBudgets(e.DetermineNamespace()).Delete(pdbName, &metav1.DeleteOptions{})
+			updated := t.generatePodDisruptionBudget(pdbName, e.Integration)
+			if _, err = t.Client.PolicyV1beta1().PodDisruptionBudgets(e.DetermineNamespace()).Create(updated); err == nil {
+				t.L.Info("PodDisruptionBudget updated: ", pdb.Spec)
+			}
+		}
+
+	} else {
+		t.L.Info("PodDisruptionBudget for the integration", e.Integration.Name, "not found")
+		pdb = t.generatePodDisruptionBudget(pdbName, e.Integration)
+		if _, err = t.Client.PolicyV1beta1().PodDisruptionBudgets(e.DetermineNamespace()).Create(pdb); err == nil {
+			t.L.Error(err, "error during creating pdb", err)
+		}
+	}
+	return err
+}
+
+// check if configuration for pdb has changed, if yes update values in pdbtrait
+func (t *pdbTrait) havePdbConfChanged(pdb *v1beta1.PodDisruptionBudget) bool {
+	return !cmp.Equal(t.MaxUnavailable, pdb.Spec.MaxUnavailable) || !cmp.Equal(t.MinAvailable, pdb.Spec.MinAvailable)
+}
+func (t *pdbTrait) generatePodDisruptionBudget(pdbName string, integration *v1.Integration) *v1beta1.PodDisruptionBudget {
+	controller := true
+	blockOwnerDeletion := true
+	spec := v1beta1.PodDisruptionBudgetSpec{
+		Selector: &metav1.LabelSelector{
+			MatchLabels: map[string]string{
+				"camel.apache.org/integration": integration.Name,
+			},
+		},
+	}
+	var min, max intstr.IntOrString
+
+	if t.MinAvailable == "" && t.MaxUnavailable == "" {
+		//set the default
+		max = intstr.Parse("1")
+		spec.MaxUnavailable = &max
+	} else if t.MinAvailable != "" {
+		// sets to maxUnavailable to nil because only one parameter is allowed to be set
+		min = intstr.Parse(t.MinAvailable)
+		spec.MinAvailable = &min
+		spec.MaxUnavailable = nil
+		t.MaxUnavailable = ""
+	} else {
+		//set maxAvailable
+		max = intstr.Parse(t.MaxUnavailable)
+		spec.MaxUnavailable = &max
+	}
+
+	return &v1beta1.PodDisruptionBudget{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      pdbName,
+			Namespace: integration.Namespace,
+			Labels:    integration.Labels,
+			OwnerReferences: []metav1.OwnerReference{

Review comment:
       This can be removed once the PDB is added to `environment.Resources`, as the _owner_ trait is responsible to set ownership for the environment resources.




----------------------------------------------------------------
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.

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



[GitHub] [camel-k] mmelko commented on pull request #1787: WIP: Create PodDisruptionBudget for every integration

Posted by GitBox <gi...@apache.org>.
mmelko commented on pull request #1787:
URL: https://github.com/apache/camel-k/pull/1787#issuecomment-716613783


   @astefanutti @lburgazzoli @nicolaferraro FYI


----------------------------------------------------------------
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.

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



[GitHub] [camel-k] astefanutti commented on a change in pull request #1787: WIP: Create PodDisruptionBudget for every integration

Posted by GitBox <gi...@apache.org>.
astefanutti commented on a change in pull request #1787:
URL: https://github.com/apache/camel-k/pull/1787#discussion_r512063252



##########
File path: pkg/trait/pdb.go
##########
@@ -0,0 +1,103 @@
+package trait
+
+import (
+	v1 "github.com/apache/camel-k/pkg/apis/camel/v1"
+	"github.com/google/go-cmp/cmp"
+	"k8s.io/api/policy/v1beta1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/intstr"
+)
+
+type pdbTrait struct {
+	BaseTrait `property:",squash"`
+	MaxUnavailable string `property:"max-unavailable" json:"maxUnavailable,omitempty"`
+	MinAvailable   string `property:"min-available" json:"minAvailable,omitempty"`
+}
+
+func newPdbTrait() Trait {
+	return &pdbTrait{
+		BaseTrait: NewBaseTrait("pdb", 900),
+	}
+}
+
+func (t *pdbTrait) Configure(e *Environment) (bool, error) {
+	return e.IntegrationInPhase(

Review comment:
       The `Configure` method should be truthy when the integration is in phases `Deploying` and `Running`, and if the trait is explicitly enabled.

##########
File path: pkg/trait/pdb.go
##########
@@ -0,0 +1,103 @@
+package trait
+
+import (
+	v1 "github.com/apache/camel-k/pkg/apis/camel/v1"
+	"github.com/google/go-cmp/cmp"
+	"k8s.io/api/policy/v1beta1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/intstr"
+)
+
+type pdbTrait struct {
+	BaseTrait `property:",squash"`
+	MaxUnavailable string `property:"max-unavailable" json:"maxUnavailable,omitempty"`
+	MinAvailable   string `property:"min-available" json:"minAvailable,omitempty"`
+}
+
+func newPdbTrait() Trait {
+	return &pdbTrait{
+		BaseTrait: NewBaseTrait("pdb", 900),
+	}
+}
+
+func (t *pdbTrait) Configure(e *Environment) (bool, error) {
+	return e.IntegrationInPhase(
+		v1.IntegrationPhaseRunning,
+	), nil
+}
+
+func (t *pdbTrait) Apply(e *Environment) error {
+	pdbName := "pdb-" + e.Integration.Name + "-it"
+	var pdb *v1beta1.PodDisruptionBudget
+	var err error
+
+	if pdb, err = t.Client.PolicyV1beta1().PodDisruptionBudgets(e.DetermineNamespace()).Get(pdbName, metav1.GetOptions{}); err == nil {
+		if a := t.havePdbConfChanged(pdb); a {
+			err = t.Client.PolicyV1beta1().PodDisruptionBudgets(e.DetermineNamespace()).Delete(pdbName, &metav1.DeleteOptions{})
+			updated := t.generatePodDisruptionBudget(pdbName, e.Integration)
+			if _, err = t.Client.PolicyV1beta1().PodDisruptionBudgets(e.DetermineNamespace()).Create(updated); err == nil {
+				t.L.Info("PodDisruptionBudget updated: ", pdb.Spec)
+			}
+		}
+
+	} else {
+		t.L.Info("PodDisruptionBudget for the integration", e.Integration.Name, "not found")
+		pdb = t.generatePodDisruptionBudget(pdbName, e.Integration)
+		if _, err = t.Client.PolicyV1beta1().PodDisruptionBudgets(e.DetermineNamespace()).Create(pdb); err == nil {
+			t.L.Error(err, "error during creating pdb", err)
+		}
+	}
+	return err
+}
+
+// check if configuration for pdb has changed, if yes update values in pdbtrait
+func (t *pdbTrait) havePdbConfChanged(pdb *v1beta1.PodDisruptionBudget) bool {
+	return !cmp.Equal(t.MaxUnavailable, pdb.Spec.MaxUnavailable) || !cmp.Equal(t.MinAvailable, pdb.Spec.MinAvailable)
+}
+func (t *pdbTrait) generatePodDisruptionBudget(pdbName string, integration *v1.Integration) *v1beta1.PodDisruptionBudget {
+	controller := true
+	blockOwnerDeletion := true
+	spec := v1beta1.PodDisruptionBudgetSpec{
+		Selector: &metav1.LabelSelector{
+			MatchLabels: map[string]string{
+				"camel.apache.org/integration": integration.Name,
+			},
+		},
+	}
+	var min, max intstr.IntOrString
+
+	if t.MinAvailable == "" && t.MaxUnavailable == "" {
+		//set the default
+		max = intstr.Parse("1")
+		spec.MaxUnavailable = &max
+	} else if t.MinAvailable != "" {
+		// sets to maxUnavailable to nil because only one parameter is allowed to be set
+		min = intstr.Parse(t.MinAvailable)
+		spec.MinAvailable = &min
+		spec.MaxUnavailable = nil
+		t.MaxUnavailable = ""
+	} else {
+		//set maxAvailable
+		max = intstr.Parse(t.MaxUnavailable)
+		spec.MaxUnavailable = &max
+	}
+
+	return &v1beta1.PodDisruptionBudget{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      pdbName,
+			Namespace: integration.Namespace,
+			Labels:    integration.Labels,
+			OwnerReferences: []metav1.OwnerReference{

Review comment:
       This can be removed once the PDB is added to `environment.Resources` as the _owner_ trait is responsible to set ownership for the environment resources.

##########
File path: pkg/trait/pdb.go
##########
@@ -0,0 +1,103 @@
+package trait
+
+import (
+	v1 "github.com/apache/camel-k/pkg/apis/camel/v1"
+	"github.com/google/go-cmp/cmp"
+	"k8s.io/api/policy/v1beta1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/intstr"
+)
+
+type pdbTrait struct {
+	BaseTrait `property:",squash"`
+	MaxUnavailable string `property:"max-unavailable" json:"maxUnavailable,omitempty"`
+	MinAvailable   string `property:"min-available" json:"minAvailable,omitempty"`
+}
+
+func newPdbTrait() Trait {
+	return &pdbTrait{
+		BaseTrait: NewBaseTrait("pdb", 900),
+	}
+}
+
+func (t *pdbTrait) Configure(e *Environment) (bool, error) {
+	return e.IntegrationInPhase(
+		v1.IntegrationPhaseRunning,
+	), nil
+}
+
+func (t *pdbTrait) Apply(e *Environment) error {
+	pdbName := "pdb-" + e.Integration.Name + "-it"
+	var pdb *v1beta1.PodDisruptionBudget
+	var err error
+
+	if pdb, err = t.Client.PolicyV1beta1().PodDisruptionBudgets(e.DetermineNamespace()).Get(pdbName, metav1.GetOptions{}); err == nil {

Review comment:
       In order to avoid each trait to implement that get / create / update logic, the idea is that each trait only adds their resources into the `environment.Resources` list. Then the _deployer_ trait does that get / create / update logic generically: https://github.com/apache/camel-k/blob/530164176b6d8354a781d1dcbe1fb21c1d92386b/pkg/trait/deployer.go#L62-L116. 




----------------------------------------------------------------
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.

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



[GitHub] [camel-k] astefanutti commented on a change in pull request #1787: Create PodDisruptionBudget for every integration

Posted by GitBox <gi...@apache.org>.
astefanutti commented on a change in pull request #1787:
URL: https://github.com/apache/camel-k/pull/1787#discussion_r521990911



##########
File path: pkg/trait/pdb.go
##########
@@ -0,0 +1,111 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements.  See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License.  You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package trait
+
+import (
+	"fmt"
+	v1 "github.com/apache/camel-k/pkg/apis/camel/v1"
+	"k8s.io/api/policy/v1beta1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/intstr"
+)
+
+// The Pbd trait allows to configure the PodDisruptionBudget resource.
+//
+// +camel-k:trait=pbd
+type pdbTrait struct {
+	BaseTrait      `property:",squash"`
+	MaxUnavailable string `property:"max-unavailable" json:"maxUnavailable,omitempty"`
+	MinAvailable   string `property:"min-available" json:"minAvailable,omitempty"`
+}
+
+func newPdbTrait() Trait {
+	return &pdbTrait{
+		BaseTrait: NewBaseTrait("pdb", 900),
+	}
+}
+
+func (t *pdbTrait) Configure(e *Environment) (bool, error) {
+	if t.Enabled != nil && *t.Enabled {
+		strategy, err := e.DetermineControllerStrategy()
+		if err != nil {
+			return false, fmt.Errorf("unable to determine the controller stratedy")
+		}
+
+		if strategy == ControllerStrategyCronJob {
+			return false, fmt.Errorf("poddisruptionbudget isn't supported with cron-job controller strategy")
+		}
+
+		if t.MaxUnavailable != "" && t.MinAvailable != "" {
+			return false, fmt.Errorf("both minAvailable and maxUnavailable can't be set simultaneously")
+		}
+
+		return e.IntegrationInPhase(
+			v1.IntegrationPhaseDeploying,
+			v1.IntegrationPhaseRunning,
+		), nil
+	}
+	return false, nil
+}
+
+func (t *pdbTrait) Apply(e *Environment) error {
+	pdb, err := t.generatePodDisruptionBudget(e)
+	if err != nil {
+		return err
+	}
+	if pdb != nil {

Review comment:
       I think we can remove that check as the `pdb` is never `nil`.




----------------------------------------------------------------
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.

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



[GitHub] [camel-k] mmelko commented on a change in pull request #1787: Create PodDisruptionBudget for every integration

Posted by GitBox <gi...@apache.org>.
mmelko commented on a change in pull request #1787:
URL: https://github.com/apache/camel-k/pull/1787#discussion_r522021882



##########
File path: pkg/trait/pdb.go
##########
@@ -0,0 +1,108 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements.  See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License.  You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package trait
+
+import (
+	"fmt"
+	v1 "github.com/apache/camel-k/pkg/apis/camel/v1"
+	"k8s.io/api/policy/v1beta1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/intstr"
+)
+
+// The Pbd trait allows to configure the PodDisruptionBudget resource.
+//
+// +camel-k:trait=pbd

Review comment:
       aaah nice catch




----------------------------------------------------------------
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.

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



[GitHub] [camel-k] mmelko commented on a change in pull request #1787: WIP: Create PodDisruptionBudget for every integration

Posted by GitBox <gi...@apache.org>.
mmelko commented on a change in pull request #1787:
URL: https://github.com/apache/camel-k/pull/1787#discussion_r517180694



##########
File path: deploy/operator-role-olm.yaml
##########
@@ -47,6 +47,15 @@ rules:
   - patch
   - update
   - watch
+- apiGroups:
+  - "policy"
+  resources:
+  - poddisruptionbudgets
+  verbs:
+  - create
+  - delete
+  - get
+  - update

Review comment:
       actually `list` is missing too.. 




----------------------------------------------------------------
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.

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



[GitHub] [camel-k] astefanutti commented on a change in pull request #1787: Create PodDisruptionBudget for every integration

Posted by GitBox <gi...@apache.org>.
astefanutti commented on a change in pull request #1787:
URL: https://github.com/apache/camel-k/pull/1787#discussion_r522005803



##########
File path: pkg/trait/pdb_test.go
##########
@@ -0,0 +1,124 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements.  See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License.  You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package trait
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	appsv1 "k8s.io/api/apps/v1"
+	corev1 "k8s.io/api/core/v1"
+	"k8s.io/api/policy/v1beta1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	v1 "github.com/apache/camel-k/pkg/apis/camel/v1"
+	"github.com/apache/camel-k/pkg/util/kubernetes"
+)
+
+func TestConfigurePdbTraitDoesSucceed(t *testing.T) {
+	pdbTrait, environment, _ := createPdbTest()
+	configured, err := pdbTrait.Configure(environment)
+
+	assert.True(t, configured)
+	assert.Nil(t, err)
+}
+
+func TestConfigurePdbTraitDoesNotSucceed(t *testing.T) {
+	pdbTrait, environment, _ := createPdbTest()
+	//err := pdbTrait.Apply(environment)

Review comment:
       Can be removed.




----------------------------------------------------------------
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.

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



[GitHub] [camel-k] astefanutti commented on a change in pull request #1787: Create PodDisruptionBudget for every integration

Posted by GitBox <gi...@apache.org>.
astefanutti commented on a change in pull request #1787:
URL: https://github.com/apache/camel-k/pull/1787#discussion_r522005278



##########
File path: pkg/trait/pdb.go
##########
@@ -0,0 +1,108 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements.  See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License.  You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package trait
+
+import (
+	"fmt"
+	v1 "github.com/apache/camel-k/pkg/apis/camel/v1"
+	"k8s.io/api/policy/v1beta1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/intstr"
+)
+
+// The Pbd trait allows to configure the PodDisruptionBudget resource.
+//
+// +camel-k:trait=pbd
+type pdbTrait struct {
+	BaseTrait      `property:",squash"`
+	MaxUnavailable string `property:"max-unavailable" json:"maxUnavailable,omitempty"`
+	MinAvailable   string `property:"min-available" json:"minAvailable,omitempty"`
+}
+
+func newPdbTrait() Trait {
+	return &pdbTrait{
+		BaseTrait: NewBaseTrait("pdb", 900),
+	}
+}
+
+func (t *pdbTrait) Configure(e *Environment) (bool, error) {
+	if t.Enabled != nil && *t.Enabled {

Review comment:
       Do you mind inverting the logic so that the bulk of the method is not in an `if` block?




----------------------------------------------------------------
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.

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



[GitHub] [camel-k] astefanutti edited a comment on pull request #1787: Create PodDisruptionBudget for every integration

Posted by GitBox <gi...@apache.org>.
astefanutti edited a comment on pull request #1787:
URL: https://github.com/apache/camel-k/pull/1787#issuecomment-722520633


   @lburgazzoli @nicolaferraro 👋,
   
   That PR introduces the _pod_ trait, whose purpose is to be responsible for all the aspects that directly relate to the integration pods configuration. That includes upcoming work to support:
   - Tolerations
   - Pod topology spread
   - Pod template, either inlined or by referencing a ConfigMap, addressing #1657.
   
   Technically, the _affinity_ trait should be merged into that new trait, yet we may want to keep that concern separated, at least to maintain compatibility for the 1.x lifespan.
   
   It turns out we already have traits responsible for the container and deployment aspects, but none for those of the pods. The _deployment_ trait could have been a candidate to meet that need. However it's specific to the _Deployment_ strategy, while the pod aspects can be shared across deployment strategies.
   
   WDYT?


----------------------------------------------------------------
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.

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



[GitHub] [camel-k] astefanutti edited a comment on pull request #1787: Create PodDisruptionBudget for every integration

Posted by GitBox <gi...@apache.org>.
astefanutti edited a comment on pull request #1787:
URL: https://github.com/apache/camel-k/pull/1787#issuecomment-722520633


   @lburgazzoli @nicolaferraro 👋,
   
   That PR introduces the _pod_ trait, whose purpose is to be responsible for all the aspects that directly relate to the integration pods configuration. That includes upcoming work to support:
   - Tolerations
   - Pod topology spread
   - Pod template, either inlined or by referencing a ConfigMap, addressing ~#1685~ #1657.
   
   Technically, the _affinity_ trait should be merged into that new trait, yet we may want to keep that concern separated, at least to maintain compatibility for the 1.x lifespan.
   
   It turns out we already have traits responsible for the container and deployment aspects, but none for those of the pods. The _deployment_ trait could have been a candidate to meet that need. However it's specific to the _Deployment_ strategy, while the pod aspects can be shared across deployment strategies.
   
   WDYT?


----------------------------------------------------------------
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.

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



[GitHub] [camel-k] astefanutti merged pull request #1787: Create PodDisruptionBudget for every integration

Posted by GitBox <gi...@apache.org>.
astefanutti merged pull request #1787:
URL: https://github.com/apache/camel-k/pull/1787


   


----------------------------------------------------------------
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.

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



[GitHub] [camel-k] astefanutti commented on pull request #1787: Create PodDisruptionBudget for every integration

Posted by GitBox <gi...@apache.org>.
astefanutti commented on pull request #1787:
URL: https://github.com/apache/camel-k/pull/1787#issuecomment-722526445


   > I guess this also relates to: #1657
   
   Damn auto-complete, that's the one I wanted to type. Corrected my previous comment.


----------------------------------------------------------------
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.

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



[GitHub] [camel-k] astefanutti commented on a change in pull request #1787: WIP: Create PodDisruptionBudget for every integration

Posted by GitBox <gi...@apache.org>.
astefanutti commented on a change in pull request #1787:
URL: https://github.com/apache/camel-k/pull/1787#discussion_r517153678



##########
File path: deploy/operator-role-olm.yaml
##########
@@ -47,6 +47,15 @@ rules:
   - patch
   - update
   - watch
+- apiGroups:
+  - "policy"
+  resources:
+  - poddisruptionbudgets
+  verbs:
+  - create
+  - delete
+  - get
+  - update

Review comment:
       `patch` verb is probably to be added.
   
   Also, `operator-role-kubernetes.yaml` must be updated.

##########
File path: pkg/trait/pod.go
##########
@@ -0,0 +1,100 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements.  See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License.  You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package trait
+
+import (
+	"fmt"
+	v1 "github.com/apache/camel-k/pkg/apis/camel/v1"
+	"k8s.io/api/policy/v1beta1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/intstr"
+)
+
+// The PodDisruptionBudget trait is used to configure the pdb
+//
+// +camel-k:trait=pod
+type podTrait struct {
+	BaseTrait      `property:",squash"`
+	MaxUnavailable string `property:"max-unavailable" json:"maxUnavailable,omitempty"`
+	MinAvailable   string `property:"min-available" json:"minAvailable,omitempty"`
+}
+
+func newPodTrait() Trait {
+	return &podTrait{
+		BaseTrait: NewBaseTrait("pod", 900),
+	}
+}
+
+func (t *podTrait) Configure(e *Environment) (bool, error) {
+	if t.Enabled != nil && !*t.Enabled {
+		return false, nil
+	}
+
+	return e.IntegrationInPhase(
+		v1.IntegrationPhaseDeploying,
+		v1.IntegrationPhaseRunning,
+	), nil
+}
+
+func (t *podTrait) Apply(e *Environment) error {
+	pdb, err := t.generatePodDisruptionBudget(e.Integration)

Review comment:
       The idiomatic way is to check the `err` right after the method call, and return `nil` at the end of the method.

##########
File path: pkg/trait/pod_test.go
##########
@@ -0,0 +1,121 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements.  See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License.  You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package trait
+
+import (
+	"k8s.io/api/policy/v1beta1"

Review comment:
       Nit: group the import with others from the same TLD.

##########
File path: deploy/operator-role-openshift.yaml
##########
@@ -47,6 +47,15 @@ rules:
   - patch
   - update
   - watch
+- apiGroups:
+    - "policy"
+  resources:
+    - poddisruptionbudgets
+  verbs:
+    - create
+    - delete
+    - get
+    - update

Review comment:
       `patch` verb is probably to be added.




----------------------------------------------------------------
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.

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



[GitHub] [camel-k] astefanutti commented on pull request #1787: WIP: Create PodDisruptionBudget for every integration

Posted by GitBox <gi...@apache.org>.
astefanutti commented on pull request #1787:
URL: https://github.com/apache/camel-k/pull/1787#issuecomment-720578410






----------------------------------------------------------------
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.

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



[GitHub] [camel-k] lburgazzoli commented on pull request #1787: Create PodDisruptionBudget for every integration

Posted by GitBox <gi...@apache.org>.
lburgazzoli commented on pull request #1787:
URL: https://github.com/apache/camel-k/pull/1787#issuecomment-722527346


   I'm thinking if after we'll have https://github.com/apache/camel-k/issues/1657, we should reduce the number of traits to the bare minimum and with a very opinionated list of what can be configured via traits and leave everything else to pod templates & co
   


----------------------------------------------------------------
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.

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



[GitHub] [camel-k] astefanutti commented on a change in pull request #1787: WIP: Create PodDisruptionBudget for every integration

Posted by GitBox <gi...@apache.org>.
astefanutti commented on a change in pull request #1787:
URL: https://github.com/apache/camel-k/pull/1787#discussion_r515003366



##########
File path: pkg/trait/pod.go
##########
@@ -0,0 +1,97 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements.  See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License.  You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package trait
+
+import (
+	v1 "github.com/apache/camel-k/pkg/apis/camel/v1"
+	"k8s.io/api/policy/v1beta1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/intstr"
+)
+
+// The PodDisruptionBudget trait is used to configure the pdb

Review comment:
       The comment should be updated accordingly to the trait name.

##########
File path: pkg/trait/pod.go
##########
@@ -0,0 +1,97 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements.  See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License.  You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package trait
+
+import (
+	v1 "github.com/apache/camel-k/pkg/apis/camel/v1"
+	"k8s.io/api/policy/v1beta1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/intstr"
+)
+
+// The PodDisruptionBudget trait is used to configure the pdb
+//
+// +camel-k:trait=pod
+type podTrait struct {
+	BaseTrait      `property:",squash"`
+	MaxUnavailable string `property:"max-unavailable" json:"maxUnavailable,omitempty"`
+	MinAvailable   string `property:"min-available" json:"minAvailable,omitempty"`
+}
+
+func newPodTrait() Trait {
+	return &podTrait{
+		BaseTrait: NewBaseTrait("pod", 900),
+	}
+}
+
+func (t *podTrait) Configure(e *Environment) (bool, error) {
+	if t.Enabled != nil && !*t.Enabled {
+		return false, nil
+	}
+
+	return e.IntegrationInPhase(
+		v1.IntegrationPhaseDeploying,
+		v1.IntegrationPhaseRunning,
+	), nil
+}
+
+func (t *podTrait) Apply(e *Environment) error {
+	pdbName := "pdb-" + e.Integration.Name + "-it"
+	t.L.Info("PodDisruptionBudget for the integration", e.Integration.Name, "not found")

Review comment:
       I'd suggest to use the Integration name for the PDB. Also the log statement can be removed.

##########
File path: pkg/trait/pod.go
##########
@@ -0,0 +1,97 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements.  See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License.  You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package trait
+
+import (
+	v1 "github.com/apache/camel-k/pkg/apis/camel/v1"
+	"k8s.io/api/policy/v1beta1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/intstr"
+)
+
+// The PodDisruptionBudget trait is used to configure the pdb
+//
+// +camel-k:trait=pod
+type podTrait struct {
+	BaseTrait      `property:",squash"`
+	MaxUnavailable string `property:"max-unavailable" json:"maxUnavailable,omitempty"`
+	MinAvailable   string `property:"min-available" json:"minAvailable,omitempty"`
+}
+
+func newPodTrait() Trait {
+	return &podTrait{
+		BaseTrait: NewBaseTrait("pod", 900),
+	}
+}
+
+func (t *podTrait) Configure(e *Environment) (bool, error) {
+	if t.Enabled != nil && !*t.Enabled {
+		return false, nil
+	}
+
+	return e.IntegrationInPhase(
+		v1.IntegrationPhaseDeploying,
+		v1.IntegrationPhaseRunning,
+	), nil
+}
+
+func (t *podTrait) Apply(e *Environment) error {
+	pdbName := "pdb-" + e.Integration.Name + "-it"
+	t.L.Info("PodDisruptionBudget for the integration", e.Integration.Name, "not found")
+	pdb := t.generatePodDisruptionBudget(pdbName, e.Integration)
+	e.Resources.Add(pdb)
+
+	return nil
+}
+
+func (t *podTrait) generatePodDisruptionBudget(pdbName string, integration *v1.Integration) *v1beta1.PodDisruptionBudget {
+	spec := v1beta1.PodDisruptionBudgetSpec{
+		Selector: &metav1.LabelSelector{
+			MatchLabels: map[string]string{
+				"camel.apache.org/integration": integration.Name,
+			},
+		},
+	}
+
+	var min, max intstr.IntOrString
+
+	if t.MinAvailable == "" && t.MaxUnavailable == "" {

Review comment:
       I'd suggest not to default values for PDB. I see this rather as an advanced feature, that is leveraged by SRE.

##########
File path: pkg/trait/pod.go
##########
@@ -0,0 +1,97 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements.  See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License.  You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package trait
+
+import (
+	v1 "github.com/apache/camel-k/pkg/apis/camel/v1"
+	"k8s.io/api/policy/v1beta1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/intstr"
+)
+
+// The PodDisruptionBudget trait is used to configure the pdb
+//
+// +camel-k:trait=pod
+type podTrait struct {
+	BaseTrait      `property:",squash"`
+	MaxUnavailable string `property:"max-unavailable" json:"maxUnavailable,omitempty"`
+	MinAvailable   string `property:"min-available" json:"minAvailable,omitempty"`
+}
+
+func newPodTrait() Trait {
+	return &podTrait{
+		BaseTrait: NewBaseTrait("pod", 900),
+	}
+}
+
+func (t *podTrait) Configure(e *Environment) (bool, error) {
+	if t.Enabled != nil && !*t.Enabled {
+		return false, nil
+	}
+
+	return e.IntegrationInPhase(
+		v1.IntegrationPhaseDeploying,
+		v1.IntegrationPhaseRunning,
+	), nil
+}
+
+func (t *podTrait) Apply(e *Environment) error {
+	pdbName := "pdb-" + e.Integration.Name + "-it"
+	t.L.Info("PodDisruptionBudget for the integration", e.Integration.Name, "not found")
+	pdb := t.generatePodDisruptionBudget(pdbName, e.Integration)
+	e.Resources.Add(pdb)

Review comment:
       I'd proposed that a PDB is created only if at least one of the two parameters is set.

##########
File path: pkg/trait/pod.go
##########
@@ -0,0 +1,97 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements.  See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License.  You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package trait
+
+import (
+	v1 "github.com/apache/camel-k/pkg/apis/camel/v1"
+	"k8s.io/api/policy/v1beta1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/intstr"
+)
+
+// The PodDisruptionBudget trait is used to configure the pdb
+//
+// +camel-k:trait=pod
+type podTrait struct {
+	BaseTrait      `property:",squash"`
+	MaxUnavailable string `property:"max-unavailable" json:"maxUnavailable,omitempty"`
+	MinAvailable   string `property:"min-available" json:"minAvailable,omitempty"`
+}
+
+func newPodTrait() Trait {
+	return &podTrait{
+		BaseTrait: NewBaseTrait("pod", 900),
+	}
+}
+
+func (t *podTrait) Configure(e *Environment) (bool, error) {
+	if t.Enabled != nil && !*t.Enabled {
+		return false, nil
+	}
+
+	return e.IntegrationInPhase(
+		v1.IntegrationPhaseDeploying,
+		v1.IntegrationPhaseRunning,
+	), nil
+}
+
+func (t *podTrait) Apply(e *Environment) error {
+	pdbName := "pdb-" + e.Integration.Name + "-it"
+	t.L.Info("PodDisruptionBudget for the integration", e.Integration.Name, "not found")
+	pdb := t.generatePodDisruptionBudget(pdbName, e.Integration)
+	e.Resources.Add(pdb)
+
+	return nil
+}
+
+func (t *podTrait) generatePodDisruptionBudget(pdbName string, integration *v1.Integration) *v1beta1.PodDisruptionBudget {
+	spec := v1beta1.PodDisruptionBudgetSpec{
+		Selector: &metav1.LabelSelector{
+			MatchLabels: map[string]string{
+				"camel.apache.org/integration": integration.Name,

Review comment:
       the `IntegrationLabel` variable can be used instead of duplicating the label name.




----------------------------------------------------------------
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.

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



[GitHub] [camel-k] astefanutti commented on a change in pull request #1787: WIP: Create PodDisruptionBudget for every integration

Posted by GitBox <gi...@apache.org>.
astefanutti commented on a change in pull request #1787:
URL: https://github.com/apache/camel-k/pull/1787#discussion_r516092283



##########
File path: pkg/trait/pod.go
##########
@@ -0,0 +1,100 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements.  See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License.  You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package trait
+
+import (
+	"fmt"
+	v1 "github.com/apache/camel-k/pkg/apis/camel/v1"
+	"k8s.io/api/policy/v1beta1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/intstr"
+)
+
+// The PodDisruptionBudget trait is used to configure the pdb
+//
+// +camel-k:trait=pod
+type podTrait struct {
+	BaseTrait      `property:",squash"`
+	MaxUnavailable string `property:"max-unavailable" json:"maxUnavailable,omitempty"`
+	MinAvailable   string `property:"min-available" json:"minAvailable,omitempty"`
+}
+
+func newPodTrait() Trait {
+	return &podTrait{
+		BaseTrait: NewBaseTrait("pod", 900),
+	}
+}
+
+func (t *podTrait) Configure(e *Environment) (bool, error) {
+	if t.Enabled != nil && !*t.Enabled {
+		return false, nil
+	}
+
+	return e.IntegrationInPhase(
+		v1.IntegrationPhaseDeploying,
+		v1.IntegrationPhaseRunning,
+	), nil
+}
+
+func (t *podTrait) Apply(e *Environment) error {
+	var err error
+	var pdb *v1beta1.PodDisruptionBudget

Review comment:
       I think quick assignment can be used instead of declaring variables.




----------------------------------------------------------------
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.

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



[GitHub] [camel-k] astefanutti commented on pull request #1787: Create PodDisruptionBudget for every integration

Posted by GitBox <gi...@apache.org>.
astefanutti commented on pull request #1787:
URL: https://github.com/apache/camel-k/pull/1787#issuecomment-723860997


   After thinking a bit based on this discussion, I'd suggest we introduce a `pdb` trait for the sole scope of that PR, and start working on the new `pod` trait to address #1657, and see what other options it could be responsible for.


----------------------------------------------------------------
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.

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



[GitHub] [camel-k] astefanutti commented on a change in pull request #1787: Create PodDisruptionBudget for every integration

Posted by GitBox <gi...@apache.org>.
astefanutti commented on a change in pull request #1787:
URL: https://github.com/apache/camel-k/pull/1787#discussion_r522004680



##########
File path: pkg/trait/pdb.go
##########
@@ -0,0 +1,108 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements.  See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License.  You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package trait
+
+import (
+	"fmt"
+	v1 "github.com/apache/camel-k/pkg/apis/camel/v1"
+	"k8s.io/api/policy/v1beta1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/intstr"
+)
+
+// The Pbd trait allows to configure the PodDisruptionBudget resource.
+//
+// +camel-k:trait=pbd

Review comment:
       `pbd` -> `pdb`:) 




----------------------------------------------------------------
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.

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



[GitHub] [camel-k] astefanutti edited a comment on pull request #1787: Create PodDisruptionBudget for every integration

Posted by GitBox <gi...@apache.org>.
astefanutti edited a comment on pull request #1787:
URL: https://github.com/apache/camel-k/pull/1787#issuecomment-722520633


   @lburgazzoli @nicolaferraro 👋,
   
   That PR introduces the _pod_ trait, whose purpose is to be responsible for all the aspects that directly relate to the integration pods configuration. That includes upcoming work to support:
   - Tolerations
   - Pod topology spread
   - Pod template, either inlined or by referencing a ConfigMap, addressing #1685.
   
   Technically, the _affinity_ trait should be merged into that new trait, yet we may want to keep that concern separated, at least to maintain compatibility for the 1.x lifespan.
   
   It turns out we already have traits responsible for the container and deployment aspects, but none for those of the pods. The _deployment_ trait could have been a candidate to meet that need. However it's specific to the _Deployment_ strategy, while the pod aspects can be shared across deployment strategies.
   
   WDYT?


----------------------------------------------------------------
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.

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



[GitHub] [camel-k] mmelko commented on pull request #1787: Create PodDisruptionBudget for every integration

Posted by GitBox <gi...@apache.org>.
mmelko commented on pull request #1787:
URL: https://github.com/apache/camel-k/pull/1787#issuecomment-725505929


   I refactored it back to `pdb` trait. 


----------------------------------------------------------------
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.

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



[GitHub] [camel-k] mmelko commented on a change in pull request #1787: Create PodDisruptionBudget for every integration

Posted by GitBox <gi...@apache.org>.
mmelko commented on a change in pull request #1787:
URL: https://github.com/apache/camel-k/pull/1787#discussion_r522002158



##########
File path: pkg/trait/pdb.go
##########
@@ -0,0 +1,111 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements.  See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License.  You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package trait
+
+import (
+	"fmt"
+	v1 "github.com/apache/camel-k/pkg/apis/camel/v1"
+	"k8s.io/api/policy/v1beta1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/intstr"
+)
+
+// The Pbd trait allows to configure the PodDisruptionBudget resource.
+//
+// +camel-k:trait=pbd
+type pdbTrait struct {
+	BaseTrait      `property:",squash"`
+	MaxUnavailable string `property:"max-unavailable" json:"maxUnavailable,omitempty"`
+	MinAvailable   string `property:"min-available" json:"minAvailable,omitempty"`
+}
+
+func newPdbTrait() Trait {
+	return &pdbTrait{
+		BaseTrait: NewBaseTrait("pdb", 900),
+	}
+}
+
+func (t *pdbTrait) Configure(e *Environment) (bool, error) {
+	if t.Enabled != nil && *t.Enabled {
+		strategy, err := e.DetermineControllerStrategy()
+		if err != nil {
+			return false, fmt.Errorf("unable to determine the controller stratedy")
+		}
+
+		if strategy == ControllerStrategyCronJob {
+			return false, fmt.Errorf("poddisruptionbudget isn't supported with cron-job controller strategy")
+		}
+
+		if t.MaxUnavailable != "" && t.MinAvailable != "" {
+			return false, fmt.Errorf("both minAvailable and maxUnavailable can't be set simultaneously")
+		}
+
+		return e.IntegrationInPhase(
+			v1.IntegrationPhaseDeploying,
+			v1.IntegrationPhaseRunning,
+		), nil
+	}
+	return false, nil
+}
+
+func (t *pdbTrait) Apply(e *Environment) error {
+	pdb, err := t.generatePodDisruptionBudget(e)
+	if err != nil {
+		return err
+	}
+	if pdb != nil {

Review comment:
       done




----------------------------------------------------------------
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.

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



[GitHub] [camel-k] lburgazzoli commented on pull request #1787: Create PodDisruptionBudget for every integration

Posted by GitBox <gi...@apache.org>.
lburgazzoli commented on pull request #1787:
URL: https://github.com/apache/camel-k/pull/1787#issuecomment-722527847


   maybe also have a flag to use a pod tamplate from a local yaml


----------------------------------------------------------------
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.

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



[GitHub] [camel-k] mmelko commented on pull request #1787: WIP: Create PodDisruptionBudget for every integration

Posted by GitBox <gi...@apache.org>.
mmelko commented on pull request #1787:
URL: https://github.com/apache/camel-k/pull/1787#issuecomment-720585915


   Should that be in operator-role-kubernetes/openshift,yaml file ?


----------------------------------------------------------------
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.

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



[GitHub] [camel-k] mmelko commented on a change in pull request #1787: WIP: Create PodDisruptionBudget for every integration

Posted by GitBox <gi...@apache.org>.
mmelko commented on a change in pull request #1787:
URL: https://github.com/apache/camel-k/pull/1787#discussion_r512219029



##########
File path: pkg/trait/pdb.go
##########
@@ -0,0 +1,103 @@
+package trait
+
+import (
+	v1 "github.com/apache/camel-k/pkg/apis/camel/v1"
+	"github.com/google/go-cmp/cmp"
+	"k8s.io/api/policy/v1beta1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/intstr"
+)
+
+type pdbTrait struct {
+	BaseTrait `property:",squash"`
+	MaxUnavailable string `property:"max-unavailable" json:"maxUnavailable,omitempty"`
+	MinAvailable   string `property:"min-available" json:"minAvailable,omitempty"`
+}
+
+func newPdbTrait() Trait {
+	return &pdbTrait{
+		BaseTrait: NewBaseTrait("pdb", 900),
+	}
+}
+
+func (t *pdbTrait) Configure(e *Environment) (bool, error) {
+	return e.IntegrationInPhase(
+		v1.IntegrationPhaseRunning,
+	), nil
+}
+
+func (t *pdbTrait) Apply(e *Environment) error {
+	pdbName := "pdb-" + e.Integration.Name + "-it"
+	var pdb *v1beta1.PodDisruptionBudget
+	var err error
+
+	if pdb, err = t.Client.PolicyV1beta1().PodDisruptionBudgets(e.DetermineNamespace()).Get(pdbName, metav1.GetOptions{}); err == nil {
+		if a := t.havePdbConfChanged(pdb); a {
+			err = t.Client.PolicyV1beta1().PodDisruptionBudgets(e.DetermineNamespace()).Delete(pdbName, &metav1.DeleteOptions{})
+			updated := t.generatePodDisruptionBudget(pdbName, e.Integration)
+			if _, err = t.Client.PolicyV1beta1().PodDisruptionBudgets(e.DetermineNamespace()).Create(updated); err == nil {
+				t.L.Info("PodDisruptionBudget updated: ", pdb.Spec)
+			}
+		}
+
+	} else {
+		t.L.Info("PodDisruptionBudget for the integration", e.Integration.Name, "not found")
+		pdb = t.generatePodDisruptionBudget(pdbName, e.Integration)
+		if _, err = t.Client.PolicyV1beta1().PodDisruptionBudgets(e.DetermineNamespace()).Create(pdb); err == nil {
+			t.L.Error(err, "error during creating pdb", err)
+		}
+	}
+	return err
+}
+
+// check if configuration for pdb has changed, if yes update values in pdbtrait
+func (t *pdbTrait) havePdbConfChanged(pdb *v1beta1.PodDisruptionBudget) bool {
+	return !cmp.Equal(t.MaxUnavailable, pdb.Spec.MaxUnavailable) || !cmp.Equal(t.MinAvailable, pdb.Spec.MinAvailable)
+}
+func (t *pdbTrait) generatePodDisruptionBudget(pdbName string, integration *v1.Integration) *v1beta1.PodDisruptionBudget {
+	controller := true
+	blockOwnerDeletion := true
+	spec := v1beta1.PodDisruptionBudgetSpec{
+		Selector: &metav1.LabelSelector{
+			MatchLabels: map[string]string{
+				"camel.apache.org/integration": integration.Name,
+			},
+		},
+	}
+	var min, max intstr.IntOrString
+
+	if t.MinAvailable == "" && t.MaxUnavailable == "" {
+		//set the default
+		max = intstr.Parse("1")
+		spec.MaxUnavailable = &max
+	} else if t.MinAvailable != "" {
+		// sets to maxUnavailable to nil because only one parameter is allowed to be set
+		min = intstr.Parse(t.MinAvailable)
+		spec.MinAvailable = &min
+		spec.MaxUnavailable = nil
+		t.MaxUnavailable = ""
+	} else {
+		//set maxAvailable
+		max = intstr.Parse(t.MaxUnavailable)
+		spec.MaxUnavailable = &max
+	}
+
+	return &v1beta1.PodDisruptionBudget{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      pdbName,
+			Namespace: integration.Namespace,
+			Labels:    integration.Labels,
+			OwnerReferences: []metav1.OwnerReference{

Review comment:
       The problem was that I it was done according the labels .. but when I looked there weren't any integration labels. I have to doublecheck




----------------------------------------------------------------
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.

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



[GitHub] [camel-k] astefanutti commented on a change in pull request #1787: Create PodDisruptionBudget for every integration

Posted by GitBox <gi...@apache.org>.
astefanutti commented on a change in pull request #1787:
URL: https://github.com/apache/camel-k/pull/1787#discussion_r521901045



##########
File path: deploy/operator-role-kubernetes.yaml
##########
@@ -47,6 +47,17 @@ rules:
   - patch
   - update
   - watch
+- apiGroups:
+  - "policy"

Review comment:
       Are the double quotes necessary?

##########
File path: pkg/trait/pdb.go
##########
@@ -0,0 +1,107 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements.  See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License.  You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package trait
+
+import (
+	"fmt"
+	v1 "github.com/apache/camel-k/pkg/apis/camel/v1"
+	"k8s.io/api/policy/v1beta1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/intstr"
+)
+
+// The Pod trait allows to configure the PodDisruptionBudget resource.

Review comment:
       `Pod` -> `PDB`

##########
File path: pkg/trait/pdb.go
##########
@@ -0,0 +1,107 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements.  See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License.  You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package trait
+
+import (
+	"fmt"
+	v1 "github.com/apache/camel-k/pkg/apis/camel/v1"
+	"k8s.io/api/policy/v1beta1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/intstr"
+)
+
+// The Pod trait allows to configure the PodDisruptionBudget resource.
+//
+// +camel-k:trait=pod
+type podTrait struct {

Review comment:
       `pod` -> `pdb`

##########
File path: pkg/trait/pdb.go
##########
@@ -0,0 +1,107 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements.  See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License.  You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package trait
+
+import (
+	"fmt"
+	v1 "github.com/apache/camel-k/pkg/apis/camel/v1"
+	"k8s.io/api/policy/v1beta1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/intstr"
+)
+
+// The Pod trait allows to configure the PodDisruptionBudget resource.
+//
+// +camel-k:trait=pod
+type podTrait struct {
+	BaseTrait      `property:",squash"`
+	MaxUnavailable string `property:"max-unavailable" json:"maxUnavailable,omitempty"`
+	MinAvailable   string `property:"min-available" json:"minAvailable,omitempty"`
+}
+
+func newPodTrait() Trait {

Review comment:
       `pod` -> `pdb`

##########
File path: pkg/trait/pdb.go
##########
@@ -0,0 +1,107 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements.  See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License.  You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package trait
+
+import (
+	"fmt"
+	v1 "github.com/apache/camel-k/pkg/apis/camel/v1"
+	"k8s.io/api/policy/v1beta1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/intstr"
+)
+
+// The Pod trait allows to configure the PodDisruptionBudget resource.
+//
+// +camel-k:trait=pod

Review comment:
       `pod` -> `pdb`

##########
File path: pkg/trait/pdb.go
##########
@@ -0,0 +1,107 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements.  See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License.  You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package trait
+
+import (
+	"fmt"
+	v1 "github.com/apache/camel-k/pkg/apis/camel/v1"
+	"k8s.io/api/policy/v1beta1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/intstr"
+)
+
+// The Pod trait allows to configure the PodDisruptionBudget resource.
+//
+// +camel-k:trait=pod
+type podTrait struct {
+	BaseTrait      `property:",squash"`
+	MaxUnavailable string `property:"max-unavailable" json:"maxUnavailable,omitempty"`
+	MinAvailable   string `property:"min-available" json:"minAvailable,omitempty"`
+}
+
+func newPodTrait() Trait {
+	return &podTrait{
+		BaseTrait: NewBaseTrait("pod", 900),

Review comment:
       `pod` -> `pdb`

##########
File path: pkg/trait/pdb.go
##########
@@ -0,0 +1,107 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements.  See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License.  You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package trait
+
+import (
+	"fmt"
+	v1 "github.com/apache/camel-k/pkg/apis/camel/v1"
+	"k8s.io/api/policy/v1beta1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/intstr"
+)
+
+// The Pod trait allows to configure the PodDisruptionBudget resource.
+//
+// +camel-k:trait=pod
+type podTrait struct {
+	BaseTrait      `property:",squash"`
+	MaxUnavailable string `property:"max-unavailable" json:"maxUnavailable,omitempty"`
+	MinAvailable   string `property:"min-available" json:"minAvailable,omitempty"`
+}
+
+func newPodTrait() Trait {
+	return &podTrait{
+		BaseTrait: NewBaseTrait("pod", 900),
+	}
+}
+
+func (t *podTrait) Configure(e *Environment) (bool, error) {
+	if t.Enabled != nil && !*t.Enabled {
+		return false, nil
+	}
+
+	return e.IntegrationInPhase(
+		v1.IntegrationPhaseDeploying,
+		v1.IntegrationPhaseRunning,
+	), nil
+}
+
+func (t *podTrait) Apply(e *Environment) error {
+	pdb, err := t.generatePodDisruptionBudget(e)
+	if err != nil {
+		return err
+	}
+	if pdb != nil {
+		e.Resources.Add(pdb)
+	}
+
+	return nil
+}
+
+func (t *podTrait) generatePodDisruptionBudget(e *Environment) (*v1beta1.PodDisruptionBudget, error) {
+	if t.MaxUnavailable == "" && t.MinAvailable == "" {

Review comment:
       Would setting a default value if the trait is enabled be useful?

##########
File path: pkg/trait/pdb.go
##########
@@ -0,0 +1,107 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements.  See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License.  You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package trait
+
+import (
+	"fmt"
+	v1 "github.com/apache/camel-k/pkg/apis/camel/v1"
+	"k8s.io/api/policy/v1beta1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/intstr"
+)
+
+// The Pod trait allows to configure the PodDisruptionBudget resource.
+//
+// +camel-k:trait=pod
+type podTrait struct {
+	BaseTrait      `property:",squash"`
+	MaxUnavailable string `property:"max-unavailable" json:"maxUnavailable,omitempty"`
+	MinAvailable   string `property:"min-available" json:"minAvailable,omitempty"`
+}
+
+func newPodTrait() Trait {
+	return &podTrait{
+		BaseTrait: NewBaseTrait("pod", 900),
+	}
+}
+
+func (t *podTrait) Configure(e *Environment) (bool, error) {
+	if t.Enabled != nil && !*t.Enabled {

Review comment:
       Should the trait be disabled when `t.Enabled == nil`?




----------------------------------------------------------------
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.

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



[GitHub] [camel-k] mmelko commented on pull request #1787: Create PodDisruptionBudget for every integration

Posted by GitBox <gi...@apache.org>.
mmelko commented on pull request #1787:
URL: https://github.com/apache/camel-k/pull/1787#issuecomment-722395273


   I've tested this also with knative and cron-job deployment strategies and it works as expected. 


----------------------------------------------------------------
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.

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