Skip to content

Commit

Permalink
Fix comments in netpol status review
Browse files Browse the repository at this point in the history
  • Loading branch information
rikatz committed Mar 29, 2022
1 parent 941a896 commit 6d0b791
Show file tree
Hide file tree
Showing 7 changed files with 9 additions and 26 deletions.
4 changes: 0 additions & 4 deletions pkg/apis/networking/types.go
Expand Up @@ -228,10 +228,6 @@ const (
// NetworkPolicyConditionReasonFeatureNotSupported represents a reason where the Network Policy may not have been
// implemented in the cluster due to a lack of some feature not supported by the Network Policy provider
NetworkPolicyConditionReasonFeatureNotSupported NetworkPolicyConditionReason = "FeatureNotSupported"

// NetworkPolicyConditionReasonInvalidRule represents a reason where the Network Policy may not have been
// implemented in the cluster due to the specified rule being invalid
NetworkPolicyConditionReasonInvalidRule NetworkPolicyConditionReason = "InvalidRule"
)

// NetworkPolicyStatus describe the current state of the NetworkPolicy.
Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/networking/validation/validation_test.go
Expand Up @@ -514,7 +514,7 @@ func TestValidateNetworkPolicyStatusUpdate(t *testing.T) {
LastTransitionTime: metav1.Time{
Time: time.Now().Add(-5 * time.Minute),
},
Reason: string(networking.NetworkPolicyConditionReasonInvalidRule),
Reason: string(networking.NetworkPolicyConditionReasonFeatureNotSupported),
Message: "endport is not supported",
ObservedGeneration: 2,
},
Expand Down
5 changes: 3 additions & 2 deletions pkg/registry/networking/networkpolicy/storage/storage_test.go
Expand Up @@ -17,6 +17,9 @@ limitations under the License.
package storage

import (
"testing"
"time"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/fields"
"k8s.io/apimachinery/pkg/labels"
Expand All @@ -34,8 +37,6 @@ import (
_ "k8s.io/kubernetes/pkg/apis/networking/install"
"k8s.io/kubernetes/pkg/features"
"k8s.io/kubernetes/pkg/registry/registrytest"
"testing"
"time"
)

func newStorage(t *testing.T) (*REST, *StatusREST, *etcd3testing.EtcdTestServer) {
Expand Down
13 changes: 3 additions & 10 deletions pkg/registry/networking/networkpolicy/strategy.go
Expand Up @@ -66,8 +66,6 @@ func (networkPolicyStrategy) PrepareForCreate(ctx context.Context, obj runtime.O

if utilfeature.DefaultFeatureGate.Enabled(features.NetworkPolicyStatus) {
// Create does not set a status when operation is not directed to status subresource
// This is not feature gated, as the field is there, and we just copy it
// when the operation is over spec and not status
networkPolicy.Status = networking.NetworkPolicyStatus{}
}

Expand All @@ -87,8 +85,6 @@ func (networkPolicyStrategy) PrepareForUpdate(ctx context.Context, obj, old runt
// As soon as the FeatureGate is removed, the whole if statement should be removed as well
if utilfeature.DefaultFeatureGate.Enabled(features.NetworkPolicyStatus) || len(oldNetworkPolicy.Status.Conditions) > 0 {
// Update is not allowed to set status when the operation is not directed to status subresource
// This is not feature gated, as the field is there, and we just copy it
// when the operation is over spec and not status
newNetworkPolicy.Status = oldNetworkPolicy.Status
}

Expand Down Expand Up @@ -172,7 +168,7 @@ func (networkPolicyStatusStrategy) PrepareForUpdate(ctx context.Context, obj, ol
// As network policy status is composed only of an array of conditions, we can say that the status
// is in use if the condition array is bigger than 0.
// quoting @thockin: "we generally keep data in this case, but no updates except to clear it"
if len(newNetworkPolicy.Status.Conditions) < 1 {
if len(newNetworkPolicy.Status.Conditions) == 0 {
newNetworkPolicy.Status = networking.NetworkPolicyStatus{}
} else {
// keep the old status in case of the update is not to clear it
Expand All @@ -183,11 +179,8 @@ func (networkPolicyStatusStrategy) PrepareForUpdate(ctx context.Context, obj, ol

// ValidateUpdate is the default update validation for an end user updating status
func (networkPolicyStatusStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList {
if utilfeature.DefaultFeatureGate.Enabled(features.NetworkPolicyStatus) {
return validation.ValidateNetworkPolicyStatusUpdate(obj.(*networking.NetworkPolicy).Status,
old.(*networking.NetworkPolicy).Status, field.NewPath("status"))
}
return nil
return validation.ValidateNetworkPolicyStatusUpdate(obj.(*networking.NetworkPolicy).Status,
old.(*networking.NetworkPolicy).Status, field.NewPath("status"))
}

// WarningsOnUpdate returns warnings for the given update.
Expand Down
3 changes: 2 additions & 1 deletion pkg/registry/networking/networkpolicy/strategy_test.go
Expand Up @@ -19,11 +19,12 @@ package networkpolicy
import (
"context"
"fmt"
genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
"reflect"
"testing"
"time"

genericapirequest "k8s.io/apiserver/pkg/endpoints/request"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/intstr"
api "k8s.io/kubernetes/pkg/apis/core"
Expand Down
4 changes: 0 additions & 4 deletions staging/src/k8s.io/api/extensions/v1beta1/types.go
Expand Up @@ -1573,10 +1573,6 @@ const (
// NetworkPolicyConditionReasonFeatureNotSupported represents a reason where the Network Policy may not have been
// implemented in the cluster due to a lack of some feature not supported by the Network Policy provider
NetworkPolicyConditionReasonFeatureNotSupported NetworkPolicyConditionReason = "FeatureNotSupported"

// NetworkPolicyConditionReasonInvalidRule represents a reason where the Network Policy may not have been
// implemented in the cluster due to the specified rule being invalid
NetworkPolicyConditionReasonInvalidRule NetworkPolicyConditionReason = "InvalidRule"
)

// NetworkPolicyStatus describe the current state of the NetworkPolicy.
Expand Down
4 changes: 0 additions & 4 deletions staging/src/k8s.io/api/networking/v1/types.go
Expand Up @@ -233,10 +233,6 @@ const (
// NetworkPolicyConditionReasonFeatureNotSupported represents a reason where the Network Policy may not have been
// implemented in the cluster due to a lack of some feature not supported by the Network Policy provider
NetworkPolicyConditionReasonFeatureNotSupported NetworkPolicyConditionReason = "FeatureNotSupported"

// NetworkPolicyConditionReasonInvalidRule represents a reason where the Network Policy may not have been
// implemented in the cluster due to the specified rule being invalid
NetworkPolicyConditionReasonInvalidRule NetworkPolicyConditionReason = "InvalidRule"
)

// NetworkPolicyStatus describe the current state of the NetworkPolicy.
Expand Down

0 comments on commit 6d0b791

Please sign in to comment.