Merge pull request #112643 from SergeyKanzhelev/removeDynamicKubeletConfig

remove DynamicKubeletConfig feature gate from the code
This commit is contained in:
Kubernetes Prow Robot
2022-10-12 01:33:00 -07:00
committed by GitHub
17 changed files with 59 additions and 363 deletions

View File

@@ -476,31 +476,3 @@ func (g *Graph) DeleteVolumeAttachment(name string) {
defer g.lock.Unlock()
g.deleteVertex_locked(vaVertexType, "", name)
}
// SetNodeConfigMap sets up edges for the Node.Spec.ConfigSource.ConfigMap relationship:
//
// configmap -> node
func (g *Graph) SetNodeConfigMap(nodeName, configMapName, configMapNamespace string) {
start := time.Now()
defer func() {
graphActionsDuration.WithLabelValues("SetNodeConfigMap").Observe(time.Since(start).Seconds())
}()
g.lock.Lock()
defer g.lock.Unlock()
// TODO(mtaufen): ensure len(nodeName) > 0 in all cases (would sure be nice to have a dependently-typed language here...)
// clear edges configmaps -> node where the destination is the current node *only*
// at present, a node can only have one *direct* configmap reference at a time
g.deleteEdges_locked(configMapVertexType, nodeVertexType, "", nodeName)
// establish new edges if we have a real ConfigMap to reference
if len(configMapName) > 0 && len(configMapNamespace) > 0 {
configmapVertex := g.getOrCreateVertex_locked(configMapVertexType, configMapNamespace, configMapName)
nodeVertex := g.getOrCreateVertex_locked(nodeVertexType, "", nodeName)
e := newDestinationEdge(configmapVertex, nodeVertex, nodeVertex)
g.graph.SetEdge(e)
g.addEdgeToDestinationIndex_locked(e)
}
}

View File

@@ -17,7 +17,6 @@ limitations under the License.
package node
import (
"fmt"
"time"
"k8s.io/klog/v2"
@@ -25,11 +24,9 @@ import (
corev1 "k8s.io/api/core/v1"
storagev1 "k8s.io/api/storage/v1"
"k8s.io/apimachinery/pkg/util/wait"
utilfeature "k8s.io/apiserver/pkg/util/feature"
corev1informers "k8s.io/client-go/informers/core/v1"
storageinformers "k8s.io/client-go/informers/storage/v1"
"k8s.io/client-go/tools/cache"
"k8s.io/kubernetes/pkg/features"
)
type graphPopulator struct {
@@ -49,15 +46,6 @@ func AddGraphEventHandlers(
var hasSynced []cache.InformerSynced
if utilfeature.DefaultFeatureGate.Enabled(features.DynamicKubeletConfig) {
nodes.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
AddFunc: g.addNode,
UpdateFunc: g.updateNode,
DeleteFunc: g.deleteNode,
})
hasSynced = append(hasSynced, nodes.Informer().HasSynced)
}
pods.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
AddFunc: g.addPod,
UpdateFunc: g.updatePod,
@@ -82,62 +70,6 @@ func AddGraphEventHandlers(
go cache.WaitForNamedCacheSync("node_authorizer", wait.NeverStop, hasSynced...)
}
func (g *graphPopulator) addNode(obj interface{}) {
g.updateNode(nil, obj)
}
func (g *graphPopulator) updateNode(oldObj, obj interface{}) {
node := obj.(*corev1.Node)
var oldNode *corev1.Node
if oldObj != nil {
oldNode = oldObj.(*corev1.Node)
}
// we only set up rules for ConfigMap today, because that is the only reference type
var name, namespace string
if source := node.Spec.ConfigSource; source != nil && source.ConfigMap != nil {
name = source.ConfigMap.Name
namespace = source.ConfigMap.Namespace
}
var oldName, oldNamespace string
if oldNode != nil {
if oldSource := oldNode.Spec.ConfigSource; oldSource != nil && oldSource.ConfigMap != nil {
oldName = oldSource.ConfigMap.Name
oldNamespace = oldSource.ConfigMap.Namespace
}
}
// if Node.Spec.ConfigSource wasn't updated, nothing for us to do
if name == oldName && namespace == oldNamespace {
return
}
path := "nil"
if node.Spec.ConfigSource != nil {
path = fmt.Sprintf("%s/%s", namespace, name)
}
klog.V(4).Infof("updateNode configSource reference to %s for node %s", path, node.Name)
g.graph.SetNodeConfigMap(node.Name, name, namespace)
}
func (g *graphPopulator) deleteNode(obj interface{}) {
if tombstone, ok := obj.(cache.DeletedFinalStateUnknown); ok {
obj = tombstone.Obj
}
node, ok := obj.(*corev1.Node)
if !ok {
klog.Infof("unexpected type %T", obj)
return
}
// NOTE: We don't remove the node, because if the node is re-created not all pod -> node
// links are re-established (we don't get relevant events because the no mutations need
// to happen in the API; the state is already there).
g.graph.SetNodeConfigMap(node.Name, "", "")
}
func (g *graphPopulator) addPod(obj interface{}) {
g.updatePod(nil, obj)
}

View File

@@ -343,74 +343,4 @@ func TestIndex(t *testing.T) {
"configmap:ns/cm3": {"node:node1=1", "node:node2=1", "node:node3=1"},
"serviceAccount:ns/sa1": {"node:node1=1", "node:node2=1", "node:node3=1"},
})
// Set node->configmap references
g.SetNodeConfigMap("node1", "cm1", "ns")
g.SetNodeConfigMap("node2", "cm1", "ns")
g.SetNodeConfigMap("node3", "cm1", "ns")
g.SetNodeConfigMap("node4", "cm1", "ns")
expectGraph(map[string][]string{
"node:node1": {},
"node:node2": {},
"node:node3": {},
"node:node4": {},
"pod:ns/pod2": {"node:node2"},
"pod:ns/pod3": {"node:node3"},
"pod:ns/pod4": {"node:node1"},
"configmap:ns/cm1": {"node:node1", "node:node2", "node:node3", "node:node4", "pod:ns/pod2", "pod:ns/pod3", "pod:ns/pod4"},
"configmap:ns/cm2": {"pod:ns/pod2", "pod:ns/pod3", "pod:ns/pod4"},
"configmap:ns/cm3": {"pod:ns/pod2", "pod:ns/pod3", "pod:ns/pod4"},
"serviceAccount:ns/sa1": {"pod:ns/pod2", "pod:ns/pod3", "pod:ns/pod4"},
})
expectIndex(map[string][]string{
"configmap:ns/cm1": {"node:node1=2", "node:node2=2", "node:node3=2", "node:node4=1"},
"configmap:ns/cm2": {"node:node1=1", "node:node2=1", "node:node3=1"},
"configmap:ns/cm3": {"node:node1=1", "node:node2=1", "node:node3=1"},
"serviceAccount:ns/sa1": {"node:node1=1", "node:node2=1", "node:node3=1"},
})
// Update node->configmap reference
g.SetNodeConfigMap("node1", "cm2", "ns")
expectGraph(map[string][]string{
"node:node1": {},
"node:node2": {},
"node:node3": {},
"node:node4": {},
"pod:ns/pod2": {"node:node2"},
"pod:ns/pod3": {"node:node3"},
"pod:ns/pod4": {"node:node1"},
"configmap:ns/cm1": {"node:node2", "node:node3", "node:node4", "pod:ns/pod2", "pod:ns/pod3", "pod:ns/pod4"},
"configmap:ns/cm2": {"node:node1", "pod:ns/pod2", "pod:ns/pod3", "pod:ns/pod4"},
"configmap:ns/cm3": {"pod:ns/pod2", "pod:ns/pod3", "pod:ns/pod4"},
"serviceAccount:ns/sa1": {"pod:ns/pod2", "pod:ns/pod3", "pod:ns/pod4"},
})
expectIndex(map[string][]string{
"configmap:ns/cm1": {"node:node1=1", "node:node2=2", "node:node3=2", "node:node4=1"},
"configmap:ns/cm2": {"node:node1=2", "node:node2=1", "node:node3=1"},
"configmap:ns/cm3": {"node:node1=1", "node:node2=1", "node:node3=1"},
"serviceAccount:ns/sa1": {"node:node1=1", "node:node2=1", "node:node3=1"},
})
// Remove node->configmap reference
g.SetNodeConfigMap("node1", "", "")
g.SetNodeConfigMap("node4", "", "")
expectGraph(map[string][]string{
"node:node1": {},
"node:node2": {},
"node:node3": {},
"node:node4": {},
"pod:ns/pod2": {"node:node2"},
"pod:ns/pod3": {"node:node3"},
"pod:ns/pod4": {"node:node1"},
"configmap:ns/cm1": {"node:node2", "node:node3", "pod:ns/pod2", "pod:ns/pod3", "pod:ns/pod4"},
"configmap:ns/cm2": {"pod:ns/pod2", "pod:ns/pod3", "pod:ns/pod4"},
"configmap:ns/cm3": {"pod:ns/pod2", "pod:ns/pod3", "pod:ns/pod4"},
"serviceAccount:ns/sa1": {"pod:ns/pod2", "pod:ns/pod3", "pod:ns/pod4"},
})
expectIndex(map[string][]string{
"configmap:ns/cm1": {"node:node1=1", "node:node2=2", "node:node3=2"},
"configmap:ns/cm2": {"node:node1=1", "node:node2=1", "node:node3=1"},
"configmap:ns/cm3": {"node:node1=1", "node:node2=1", "node:node3=1"},
"serviceAccount:ns/sa1": {"node:node1=1", "node:node2=1", "node:node3=1"},
})
}

View File

@@ -30,7 +30,6 @@ import (
corev1 "k8s.io/api/core/v1"
storagev1 "k8s.io/api/storage/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apiserver/pkg/authentication/user"
"k8s.io/apiserver/pkg/authorization/authorizer"
utilfeature "k8s.io/apiserver/pkg/util/feature"
@@ -68,11 +67,6 @@ func TestAuthorizer(t *testing.T) {
expect authorizer.Decision
features featuregate.FeatureGate
}{
{
name: "allowed node configmap",
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "configmaps", Name: "node0-configmap", Namespace: "ns0"},
expect: authorizer.DecisionAllow,
},
{
name: "allowed configmap",
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "configmaps", Name: "configmap0-pod0-node0", Namespace: "ns0"},
@@ -128,12 +122,6 @@ func TestAuthorizer(t *testing.T) {
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "persistentvolumes", Name: "pv0-pod0-node0-ns0", Namespace: ""},
expect: authorizer.DecisionAllow,
},
{
name: "disallowed node configmap",
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "configmaps", Name: "node1-configmap", Namespace: "ns0"},
expect: authorizer.DecisionNoOpinion,
},
{
name: "disallowed configmap",
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "configmaps", Name: "configmap0-pod0-node1", Namespace: "ns0"},
@@ -385,36 +373,23 @@ func TestAuthorizerSharedResources(t *testing.T) {
}
g.AddPod(pod3)
g.SetNodeConfigMap("node1", "shared-configmap", "ns1")
g.SetNodeConfigMap("node2", "shared-configmap", "ns1")
g.SetNodeConfigMap("node3", "configmap", "ns1")
testcases := []struct {
User user.Info
Secret string
ConfigMap string
ExpectAllowed bool
User user.Info
Secret string
ConfigMap string
Decision authorizer.Decision
}{
{User: node1, ExpectAllowed: true, Secret: "node1-only"},
{User: node1, ExpectAllowed: true, Secret: "node1-node2-only"},
{User: node1, ExpectAllowed: true, Secret: "shared-all"},
{User: node1, Decision: authorizer.DecisionAllow, Secret: "node1-only"},
{User: node1, Decision: authorizer.DecisionAllow, Secret: "node1-node2-only"},
{User: node1, Decision: authorizer.DecisionAllow, Secret: "shared-all"},
{User: node2, ExpectAllowed: false, Secret: "node1-only"},
{User: node2, ExpectAllowed: true, Secret: "node1-node2-only"},
{User: node2, ExpectAllowed: true, Secret: "shared-all"},
{User: node2, Decision: authorizer.DecisionNoOpinion, Secret: "node1-only"},
{User: node2, Decision: authorizer.DecisionAllow, Secret: "node1-node2-only"},
{User: node2, Decision: authorizer.DecisionAllow, Secret: "shared-all"},
{User: node3, ExpectAllowed: false, Secret: "node1-only"},
{User: node3, ExpectAllowed: false, Secret: "node1-node2-only"},
{User: node3, ExpectAllowed: true, Secret: "shared-all"},
{User: node1, ExpectAllowed: true, ConfigMap: "shared-configmap"},
{User: node1, ExpectAllowed: false, ConfigMap: "configmap"},
{User: node2, ExpectAllowed: true, ConfigMap: "shared-configmap"},
{User: node2, ExpectAllowed: false, ConfigMap: "configmap"},
{User: node3, ExpectAllowed: false, ConfigMap: "shared-configmap"},
{User: node3, ExpectAllowed: true, ConfigMap: "configmap"},
{User: node3, Decision: authorizer.DecisionNoOpinion, Secret: "node1-only"},
{User: node3, Decision: authorizer.DecisionNoOpinion, Secret: "node1-node2-only"},
{User: node3, Decision: authorizer.DecisionAllow, Secret: "shared-all"},
}
for i, tc := range testcases {
@@ -439,8 +414,8 @@ func TestAuthorizerSharedResources(t *testing.T) {
t.Fatalf("test case must include a request for a Secret or ConfigMap")
}
if (decision == authorizer.DecisionAllow) != tc.ExpectAllowed {
t.Errorf("%d: expected %v, got %v", i, tc.ExpectAllowed, decision)
if decision != tc.Decision {
t.Errorf("%d: expected %v, got %v", i, tc.Decision, decision)
}
}
@@ -629,11 +604,6 @@ func BenchmarkAuthorization(b *testing.B) {
expect authorizer.Decision
features featuregate.FeatureGate
}{
{
name: "allowed node configmap",
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "configmaps", Name: "node0-configmap", Namespace: "ns0"},
expect: authorizer.DecisionAllow,
},
{
name: "allowed configmap",
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "configmaps", Name: "configmap0-pod0-node0", Namespace: "ns0"},
@@ -649,12 +619,6 @@ func BenchmarkAuthorization(b *testing.B) {
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "secrets", Name: "secret0-shared", Namespace: "ns0"},
expect: authorizer.DecisionAllow,
},
{
name: "disallowed node configmap",
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "configmaps", Name: "node1-configmap", Namespace: "ns0"},
expect: authorizer.DecisionNoOpinion,
},
{
name: "disallowed configmap",
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "configmaps", Name: "configmap0-pod0-node1", Namespace: "ns0"},
@@ -779,9 +743,6 @@ func BenchmarkAuthorization(b *testing.B) {
func populate(graph *Graph, nodes []*corev1.Node, pods []*corev1.Pod, pvs []*corev1.PersistentVolume, attachments []*storagev1.VolumeAttachment) {
p := &graphPopulator{}
p.graph = graph
for _, node := range nodes {
p.addNode(node)
}
for _, pod := range pods {
p.addPod(pod)
}
@@ -830,19 +791,9 @@ func generate(opts *sampleDataOpts) ([]*corev1.Node, []*corev1.Pod, []*corev1.Pe
attachments = append(attachments, attachment)
}
name := fmt.Sprintf("%s-configmap", nodeName)
nodes = append(nodes, &corev1.Node{
ObjectMeta: metav1.ObjectMeta{Name: nodeName},
Spec: corev1.NodeSpec{
ConfigSource: &corev1.NodeConfigSource{
ConfigMap: &corev1.ConfigMapNodeConfigSource{
Name: name,
Namespace: "ns0",
UID: types.UID(fmt.Sprintf("ns0-%s", name)),
KubeletConfigKey: "kubelet",
},
},
},
Spec: corev1.NodeSpec{},
})
}
return nodes, pods, pvs, attachments