Reflect comments

Signed-off-by: naoki-take <[email protected]>
cybozu-go · Apr 4, 2024 · df7a7c3 · df7a7c3
1 parent 1c06696
commit df7a7c3
Show file tree

Hide file tree

Showing 7 changed files with 19 additions and 18 deletions.
diff --git a/docs/usage.md b/docs/usage.md
@@ -173,7 +173,7 @@ Step 5 and 6 are automatically done by Coil.
 It defines an egress portal of the cluster for some destinations.
 
 Coil creates a `Deployment` and `Service` for each `Egress`.
-It also creates `PodDisruptionBudget` when `spec.podDisruptionBudget` is specified.
+It also creates a `PodDisruptionBudget` when `spec.podDisruptionBudget` is specified.
 
 Here is an example `Egress` resource for the Internet:
 
@@ -247,7 +247,7 @@ You may customize the container of egress Pods as shown in the above example.
 | `template`              | [PodTemplateSpec][]       | Copied to Deployment's `spec.template`.                              |
 | `sessionAffinity`       | `ClusterIP` or `None`     | Copied to Service's `spec.sessionAffinity`.  Default is `ClusterIP`. |
 | `sessionAffinityConfig` | [SessionAffinityConfig][] | Copied to Service's `spec.sessionAffinityConfig`.                    |
-| `podDisruptionBudget`   | `EgressPDB`               | `minAvailable` and `maxUnavailable` are supported.                   |
+| `podDisruptionBudget`   | `EgressPDBSpec`           | `minAvailable` and `maxUnavailable` are copied to PDB's spec.        |
 
 ### Client Pods
 
@@ -371,4 +371,4 @@ The example of Grafana dashboard is [here](../v2/dashboard/coil.json).
 [DeploymentStrategy]: https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.19/#deploymentstrategy-v1-apps
 [PodTemplateSpec]: https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.19/#podtemplatespec-v1-core 
 [SessionAffinityConfig]: https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.19/#sessionaffinityconfig-v1-core
-[NetworkPolicy]: https://kubernetes.io/docs/concepts/services-networking/network-policies/
+[NetworkPolicy]: https://kubernetes.io/docs/concepts/services-networking/network-policies/
diff --git a/v2/api/v2/egress_types.go b/v2/api/v2/egress_types.go
@@ -66,7 +66,7 @@ type EgressSpec struct {
 
 	// PodDisruptionBudget is an optional PodDisruptionBudget for Egress NAT pods.
 	// +optional
-	PodDisruptionBudget *EgressPDB `json:"podDisruptionBudget,omitempty"`
+	PodDisruptionBudget *EgressPDBSpec `json:"podDisruptionBudget,omitempty"`
 }
 
 // EgressPodTemplate defines pod template for Egress
@@ -84,7 +84,7 @@ type EgressPodTemplate struct {
 }
 
 // EgressPDB defines PDB for Egress
-type EgressPDB struct {
+type EgressPDBSpec struct {
 	// MinAvailable is the minimum number of pods that must be available at any given time.
 	// +optional
 	MinAvailable *intstr.IntOrString `json:"minAvailable,omitempty"`
@@ -150,7 +150,9 @@ func (es EgressSpec) validateUpdate() field.ErrorList {
 	return es.validate()
 }
 
-func validatePodDisruptionBudget(pdb EgressPDB, fldPath *field.Path) field.ErrorList {
+// For validatiion of PodDisruptionBudget
+// Ref. https://github.com/kubernetes/kubernetes/blob/master/pkg/apis/policy/validation/validation.go
+func validatePodDisruptionBudget(pdb EgressPDBSpec, fldPath *field.Path) field.ErrorList {
 	allErrs := field.ErrorList{}
 
 	if pdb.MinAvailable != nil && pdb.MaxUnavailable != nil {

diff --git a/v2/api/v2/egress_webhook_test.go b/v2/api/v2/egress_webhook_test.go
@@ -180,7 +180,7 @@ var _ = Describe("Egress Webhook", func() {
 	It("should allow valid PDB", func() {
 		r := makeEgress()
 		maxUnavailable := intstr.FromInt(1)
-		r.Spec.PodDisruptionBudget = &EgressPDB{MaxUnavailable: &maxUnavailable}
+		r.Spec.PodDisruptionBudget = &EgressPDBSpec{MaxUnavailable: &maxUnavailable}
 		err := k8sClient.Create(ctx, r)
 		Expect(err).NotTo(HaveOccurred())
 	})
@@ -189,13 +189,13 @@ var _ = Describe("Egress Webhook", func() {
 		r := makeEgress()
 		maxUnavailable := intstr.FromInt(1)
 		minAvailable := intstr.FromInt(1)
-		r.Spec.PodDisruptionBudget = &EgressPDB{MaxUnavailable: &maxUnavailable, MinAvailable: &minAvailable}
+		r.Spec.PodDisruptionBudget = &EgressPDBSpec{MaxUnavailable: &maxUnavailable, MinAvailable: &minAvailable}
 		err := k8sClient.Create(ctx, r)
 		Expect(err).To(HaveOccurred())
 
 		r = makeEgress()
 		maxUnavailable = intstr.FromString("120%")
-		r.Spec.PodDisruptionBudget = &EgressPDB{MaxUnavailable: &maxUnavailable}
+		r.Spec.PodDisruptionBudget = &EgressPDBSpec{MaxUnavailable: &maxUnavailable}
 		err = k8sClient.Create(ctx, r)
 		Expect(err).To(HaveOccurred())
 	})

diff --git a/v2/api/v2/zz_generated.deepcopy.go b/v2/api/v2/zz_generated.deepcopy.go
diff --git a/v2/controllers/egress_controller.go b/v2/controllers/egress_controller.go
@@ -390,8 +390,7 @@ func (r *EgressReconciler) reconcilePDB(ctx context.Context, log logr.Logger, eg
 		if pdb.Labels == nil {
 			pdb.Labels = make(map[string]string)
 		}
-		labels := selectorLabels(eg.Name)
-		for k, v := range labels {
+		for k, v := range selectorLabels(eg.Name) {
 			pdb.Labels[k] = v
 		}
 

diff --git a/v2/controllers/egress_controller_test.go b/v2/controllers/egress_controller_test.go
@@ -516,7 +516,7 @@ var _ = Describe("Egress reconciler", func() {
 		By("creating an Egress")
 		eg := makeEgress("eg7")
 		minAvailable := intstr.FromInt(1)
-		eg.Spec.PodDisruptionBudget = &coilv2.EgressPDB{
+		eg.Spec.PodDisruptionBudget = &coilv2.EgressPDBSpec{
 			MinAvailable: &minAvailable,
 		}
 		err := k8sClient.Create(ctx, eg)

diff --git a/v2/e2e/coil_test.go b/v2/e2e/coil_test.go
@@ -282,7 +282,7 @@ var _ = Describe("Coil", func() {
 		}).Should(Equal(2))
 
 		By("deleting Egress")
-		kubectlSafe(nil, "delete", "-f", "manifests/egress.yaml")
+		kubectlSafe(nil, "delete", "egress", "-n", "internet", "egress")
 
 		By("checking PDB deletion")
 		Eventually(func() error {