Skip to content

Commit f50570e

Browse files
vakalapahuntergregory
vakalapa
and
huntergregory
authored
fix: [NPM] Remove error on not finding server version (#1571)
* fix: [NPM] Remove error on not finding server version * removing the isnewNetpol flag * fix UTs * removing dependency on windows builds * putting pipeline win dependency back Co-authored-by: Hunter Gregory <[email protected]> Co-authored-by: Hunter Gregory <[email protected]>
1 parent cfe483b commit f50570e

File tree

9 files changed

+9
-277
lines changed

9 files changed

+9
-277
lines changed

‎npm/cmd/main.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ var version string
2626
// panicRecoverAndExitWithStackTrace - recovery from panic, print a failure message and stack trace and exit the program
2727
func panicRecoverAndExitWithStackTrace() {
2828
if r := recover(); r != nil {
29-
klog.Errorf("%+v", r)
29+
klog.Infoln(r)
3030
klog.Errorf("Stack trace: %s", string(debug.Stack()))
3131
}
3232
}

‎npm/cmd/start.go‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -177,13 +177,7 @@ func k8sServerVersion(kubeclientset kubernetes.Interface) *k8sversion.Info {
177177
}
178178

179179
if err != nil {
180-
metrics.SendErrorLogAndMetric(util.NpmID, "Error: failed to retrieving kubernetes version")
181-
panic(err.Error)
182-
}
183-
184-
if err = util.SetIsNewNwPolicyVerFlag(serverVersion); err != nil {
185-
metrics.SendErrorLogAndMetric(util.NpmID, "Error: failed to set IsNewNwPolicyVerFlag")
186-
panic(err.Error)
180+
metrics.SendErrorLogAndMetric(util.NpmID, "Error: failed to retrieving kubernetes version with err: %s", err.Error())
187181
}
188182
return serverVersion
189183
}

‎npm/cmd/start_test.go‎

Lines changed: 0 additions & 102 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,7 @@ import (
44
"testing"
55

66
"github.com/Azure/azure-container-networking/log"
7-
"github.com/Azure/azure-container-networking/npm/util"
87
"github.com/stretchr/testify/require"
9-
k8sversion "k8s.io/apimachinery/pkg/version"
10-
fakediscovery "k8s.io/client-go/discovery/fake"
11-
"k8s.io/client-go/kubernetes/fake"
128
)
139

1410
func TestInitLogging(t *testing.T) {
@@ -17,101 +13,3 @@ func TestInitLogging(t *testing.T) {
1713
require.NoError(t, err)
1814
require.Equal(t, expectedLogPath, log.GetLogDirectory())
1915
}
20-
21-
func TestK8sServerVersion(t *testing.T) {
22-
// NPM has break behavior change from k8s version >= 1.11.
23-
// Thus, util.IsNewNwPolicyVerFlag flag is set based on running K8s version.
24-
tests := []struct {
25-
name string
26-
info *k8sversion.Info
27-
wantPanic bool
28-
isNewNwPolicyVer bool
29-
}{
30-
{
31-
name: "Test higher version (>1.11)",
32-
info: &k8sversion.Info{
33-
Major: "1.20",
34-
Minor: "2",
35-
GitVersion: "v1.20.2",
36-
},
37-
wantPanic: false,
38-
isNewNwPolicyVer: true,
39-
},
40-
{
41-
name: "Test equal version (1.11)",
42-
info: &k8sversion.Info{
43-
Major: "1.11",
44-
Minor: "0",
45-
GitVersion: "v1.11",
46-
},
47-
wantPanic: false,
48-
isNewNwPolicyVer: true,
49-
},
50-
{
51-
name: "Test lower version (<1.11)",
52-
info: &k8sversion.Info{
53-
Major: "1.10",
54-
Minor: "1",
55-
GitVersion: "v1.10.1",
56-
},
57-
wantPanic: false,
58-
isNewNwPolicyVer: false,
59-
},
60-
{
61-
name: "Test lower version (<1.11)",
62-
info: &k8sversion.Info{
63-
Major: "0",
64-
Minor: "0",
65-
GitVersion: "v0.0",
66-
},
67-
wantPanic: false,
68-
isNewNwPolicyVer: false,
69-
},
70-
{
71-
name: "Test wrong minus version",
72-
info: &k8sversion.Info{
73-
Major: "-1.11",
74-
Minor: "0",
75-
GitVersion: "v-1.11",
76-
},
77-
wantPanic: true,
78-
},
79-
{
80-
name: "Test wrong alphabet version",
81-
info: &k8sversion.Info{
82-
Major: "ab",
83-
Minor: "cc",
84-
GitVersion: "vab.cc",
85-
},
86-
wantPanic: true,
87-
},
88-
{
89-
name: "Test wrong alphabet version",
90-
info: &k8sversion.Info{
91-
Major: "1.1",
92-
Minor: "cc",
93-
GitVersion: "v1.1.cc",
94-
},
95-
wantPanic: true,
96-
},
97-
}
98-
99-
fc := fake.NewSimpleClientset()
100-
for _, tt := range tests {
101-
tt := tt
102-
fc.Discovery().(*fakediscovery.FakeDiscovery).FakedServerVersion = tt.info
103-
t.Run(tt.name, func(t *testing.T) {
104-
if tt.wantPanic {
105-
require.Panics(t, func() {
106-
k8sServerVersion(fc)
107-
})
108-
} else {
109-
require.NotPanics(t, func() {
110-
got := k8sServerVersion(fc)
111-
require.Equal(t, got, tt.info)
112-
require.Equal(t, util.IsNewNwPolicyVerFlag, tt.isNewNwPolicyVer)
113-
})
114-
}
115-
})
116-
}
117-
}

‎npm/pkg/controlplane/controllers/v1/translatePolicy.go‎

Lines changed: 7 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -121,11 +121,14 @@ func craftPartialIptEntrySpecFromOpsAndLabels(ns string, ops, labels []string, s
121121
// craftPartialIptEntrySpecFromSelector :- ns must be "" for namespace selectors
122122
// func helps in taking a labelSelector and converts it into corresponding matchSets
123123
// to be a used in full iptable rules
124-
// selector *metav1.LabelSelector: is used to create matchSets
125-
// ns string: helps with adding a namespace name in case of empty (or all) selector
126-
// srcOrDstFlag string: helps with determining if the src flag is to used in matchsets or dst flag,
124+
//
125+
// selector *metav1.LabelSelector: is used to create matchSets
126+
// ns string: helps with adding a namespace name in case of empty (or all) selector
127+
// srcOrDstFlag string: helps with determining if the src flag is to used in matchsets or dst flag,
128+
//
127129
// depending on ingress or egress translate policy
128-
// isNamespaceSelector bool: helps in adding prefix for nameSpace ipsets
130+
//
131+
// isNamespaceSelector bool: helps in adding prefix for nameSpace ipsets
129132
func craftPartialIptEntrySpecFromSelector(ns string, selector *metav1.LabelSelector, srcOrDstFlag string, isNamespaceSelector bool) ([]string, []string, map[string][]string) {
130133
// parse the sector into labels and maps of multiVal match Exprs
131134
labelsWithOps, nsLabelListKVs := parseSelector(selector)
@@ -715,13 +718,6 @@ func translateIngress(ns string, policyName string, targetSelector metav1.LabelS
715718
continue
716719
}
717720

718-
// fromRule has both namespaceSelector and podSelector set.
719-
// We should match the selected pods in the selected namespaces.
720-
// This allows traffic from podSelector intersects namespaceSelector
721-
// This is only supported in kubernetes version >= 1.11
722-
if !util.IsNewNwPolicyVerFlag {
723-
continue
724-
}
725721
for _, nsSelector := range FlattenNameSpaceSelector(fromRule.NamespaceSelector) {
726722
// we pass empty ns for the podspec and comment here because it's a combo of both selectors and not limited to network policy namespace
727723
iptPartialNsSpec, nsLabelsWithoutOps, listLabelsWithMembers := craftPartialIptEntrySpecFromSelector("", &nsSelector, util.IptablesSrcFlag, true) // Add namespaces prefix to distinguish namespace ipsets and pod ipsets
@@ -1377,13 +1373,6 @@ func translateEgress(ns string, policyName string, targetSelector metav1.LabelSe
13771373
continue
13781374
}
13791375

1380-
// toRule has both namespaceSelector and podSelector set.
1381-
// We should match the selected pods in the selected namespaces.
1382-
// This allows traffic from podSelector intersects namespaceSelector
1383-
// This is only supported in kubernetes version >= 1.11
1384-
if !util.IsNewNwPolicyVerFlag {
1385-
continue
1386-
}
13871376
for _, nsSelector := range FlattenNameSpaceSelector(toRule.NamespaceSelector) {
13881377
// we pass true for the podspec and comment here because it's a combo of both selectors and not limited to network policy namespace
13891378
iptPartialNsSpec, nsLabelsWithoutOps, listLabelsWithMembers := craftPartialIptEntrySpecFromSelector("", &nsSelector, util.IptablesDstFlag, true)

‎npm/pkg/controlplane/controllers/v1/translatePolicy_test.go‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -642,7 +642,6 @@ func TestTranslateIngress(t *testing.T) {
642642
},
643643
}
644644

645-
util.IsNewNwPolicyVerFlag = true
646645
sets, _, lists, _, iptEntries := translateIngress(ns, name, targetSelector, rules)
647646
expectedSets := []string{
648647
"context:dev",
@@ -925,7 +924,6 @@ func TestTranslateEgress(t *testing.T) {
925924
},
926925
}
927926

928-
util.IsNewNwPolicyVerFlag = true
929927
sets, _, lists, _, iptEntries := translateEgress(ns, name, targetSelector, rules)
930928
expectedSets := []string{
931929
"context:dev",
@@ -1788,7 +1786,6 @@ func TestAllowNsDevAndAppBackendToAppFrontend(t *testing.T) {
17881786
t.Fatal(err)
17891787
}
17901788

1791-
util.IsNewNwPolicyVerFlag = true
17921789
sets, _, lists, _, _, iptEntries := translatePolicy(allowNsDevAndBackendToFrontendPolicy)
17931790

17941791
expectedSets := []string{
@@ -2400,8 +2397,6 @@ func TestAllowMultiplePodSelectors(t *testing.T) {
24002397
t.Fatal(err)
24012398
}
24022399
2403-
util.IsNewNwPolicyVerFlag = true
2404-
24052400
sets, _, lists, _, _, iptEntries := translatePolicy(multiPodSlector)
24062401
24072402
expectedSets := []string{

‎npm/pkg/controlplane/translation/translatePolicy.go‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -434,14 +434,6 @@ func translateRule(npmNetPol *policies.NPMNetworkPolicy, netPolName string, dire
434434
continue
435435
}
436436

437-
// peer has both namespaceSelector and podSelector set.
438-
// We should match the selected pods in the selected namespaces.
439-
// This allows traffic from podSelector intersects namespaceSelector
440-
// This is only supported in kubernetes version >= 1.11
441-
if !util.IsNewNwPolicyVerFlag {
442-
continue
443-
}
444-
445437
// #2.4 handle namespaceSelector and podSelector and port if exist
446438
psResult, err := podSelector(npmNetPol.PolicyKey, matchType, peer.PodSelector)
447439
if err != nil {

0 commit comments

Comments
 (0)