From 08f2765dada2398423c1e3567b2f7a15302a1f22 Mon Sep 17 00:00:00 2001 From: Satish Matti Date: Sat, 27 May 2023 00:25:14 -0700 Subject: [PATCH] Remove remaining node update calls in calico-node startup Followup to https://github.com/projectcalico/calico/pull/7550 Change-Id: I01c8018c9d47401383f05861865ff0c9b44c506e --- node/filesystem/etc/rc.local | 6 ++- node/pkg/lifecycle/startup/startup.go | 47 +++++++++++----------- node/pkg/lifecycle/startup/startup_test.go | 17 ++++---- 3 files changed, 37 insertions(+), 33 deletions(-) diff --git a/node/filesystem/etc/rc.local b/node/filesystem/etc/rc.local index 6ab155aa9ed..24768f9dff2 100755 --- a/node/filesystem/etc/rc.local +++ b/node/filesystem/etc/rc.local @@ -36,8 +36,10 @@ if [ -z "$CALICO_DISABLE_FELIX" ]; then cp -a /etc/service/available/felix /etc/service/enabled/ fi -# Monitor change in node IP addresses and subnets. -cp -a /etc/service/available/monitor-addresses /etc/service/enabled/ +if [ "$CALICO_NETWORKING_BACKEND" != "none" ]; then + # Monitor change in node IP addresses and subnets. + cp -a /etc/service/available/monitor-addresses /etc/service/enabled/ +fi # Enable the allocate tunnel IP service cp -a /etc/service/available/allocate-tunnel-addrs /etc/service/enabled/ diff --git a/node/pkg/lifecycle/startup/startup.go b/node/pkg/lifecycle/startup/startup.go index 77c5058902d..7d7e1c91784 100644 --- a/node/pkg/lifecycle/startup/startup.go +++ b/node/pkg/lifecycle/startup/startup.go @@ -182,25 +182,22 @@ func Run() { } } - // If Calico is running in policy only mode we don't need to write BGP related details to the Node. - if os.Getenv("CALICO_NETWORKING_BACKEND") != "none" { - configureAndCheckIPAddressSubnets(ctx, cli, node, k8sNode) - // Configure the node AS number. - configureASNumber(node) - } - + needsNodeUpdate := configureAndCheckIPAddressSubnets(ctx, cli, node, k8sNode) + // Configure the node AS number. + needsNodeUpdate = configureASNumber(node) || needsNodeUpdate // Populate a reference to the node based on orchestrator node identifiers. - configureNodeRef(node) + needsNodeUpdate = configureNodeRef(node) || needsNodeUpdate + if needsNodeUpdate { + // Apply the updated node resource. + if _, err := CreateOrUpdate(ctx, cli, node); err != nil { + log.WithError(err).Errorf("Unable to set node resource configuration") + utils.Terminate() + } + } // Check expected filesystem ensureFilesystemAsExpected() - // Apply the updated node resource. - if _, err := CreateOrUpdate(ctx, cli, node); err != nil { - log.WithError(err).Errorf("Unable to set node resource configuration") - utils.Terminate() - } - // Configure IP Pool configuration. configureIPPools(ctx, cli, kubeadmConfig) @@ -257,6 +254,11 @@ func getMonitorPollInterval() time.Duration { } func configureAndCheckIPAddressSubnets(ctx context.Context, cli client.Interface, node *libapi.Node, k8sNode *v1.Node) bool { + // If Calico is running in policy only mode we don't need to write BGP related + // details to the Node. + if os.Getenv("CALICO_NETWORKING_BACKEND") == "none" { + return false + } // Configure and verify the node IP addresses and subnets. checkConflicts, err := configureIPsAndSubnets(node, k8sNode, func(incl []string, excl []string, version int) ([]autodetection.Interface, error) { return autodetection.GetInterfaces(net.Interfaces, incl, excl, version) @@ -309,12 +311,6 @@ func configureAndCheckIPAddressSubnets(ctx context.Context, cli client.Interface } func MonitorIPAddressSubnets() { - // If Calico is running in policy only mode we don't need to write BGP - // related details to the Node. - if os.Getenv("CALICO_NETWORKING_BACKEND") == "none" { - log.Info("Skipped monitoring node IP changes when CALICO_NETWORKING_BACKEND=none") - return - } ctx := context.Background() _, cli := calicoclient.CreateClient() nodeName := utils.DetermineNodeName() @@ -369,16 +365,18 @@ func MonitorIPAddressSubnets() { // configureNodeRef will attempt to discover the cluster type it is running on, check to ensure we // have not already set it on this Node, and set it if need be. -func configureNodeRef(node *libapi.Node) { +// Returns true if the node object needs to updated. +func configureNodeRef(node *libapi.Node) bool { orchestrator := "k8s" nodeRef := "" // Sort out what type of cluster we're running on. if nodeRef = os.Getenv("CALICO_K8S_NODE_REF"); nodeRef == "" { - return + return false } node.Spec.OrchRefs = []libapi.OrchRef{{NodeName: nodeRef, Orchestrator: orchestrator}} + return true } // CreateOrUpdate creates the Node if ResourceVersion is not specified, @@ -690,7 +688,8 @@ func evaluateENVBool(envVar string, defaultValue bool) bool { // configureASNumber configures the Node resource with the AS number specified // in the environment, or is a no-op if not specified. -func configureASNumber(node *libapi.Node) { +// Returns true if the node object needs to be updated. +func configureASNumber(node *libapi.Node) bool { // Extract the AS number from the environment asStr := os.Getenv("AS") if asStr != "" { @@ -700,6 +699,7 @@ func configureASNumber(node *libapi.Node) { } else { log.Infof("Using AS number specified in environment (AS=%s)", asNum) node.Spec.BGP.ASNumber = &asNum + return true } } else { if node.Spec.BGP.ASNumber == nil { @@ -708,6 +708,7 @@ func configureASNumber(node *libapi.Node) { log.Infof("Using AS number %s configured in node resource", node.Spec.BGP.ASNumber) } } + return false } // generateIPv6ULAPrefix return a random generated ULA IPv6 prefix as per RFC 4193. The pool diff --git a/node/pkg/lifecycle/startup/startup_test.go b/node/pkg/lifecycle/startup/startup_test.go index 711b1401181..f52c80731f3 100644 --- a/node/pkg/lifecycle/startup/startup_test.go +++ b/node/pkg/lifecycle/startup/startup_test.go @@ -91,7 +91,7 @@ func makeK8sNode(ipv4 string, ipv6 string) *v1.Node { } var _ = DescribeTable("Node IP detection failure cases", - func(networkingBackend string, expectedExitCode int, rrCId string) { + func(networkingBackend string, expectedExitCode int, rrCId string, expectedUpdate bool) { os.Setenv("CALICO_NETWORKING_BACKEND", networkingBackend) os.Setenv("IP", "none") os.Setenv("IP6", "") @@ -113,16 +113,17 @@ var _ = DescribeTable("Node IP detection failure cases", node.Spec.BGP = &libapi.NodeBGPSpec{RouteReflectorClusterID: rrCId} } - _ = configureAndCheckIPAddressSubnets(context.Background(), c, &node, &v1.Node{}) + updated := configureAndCheckIPAddressSubnets(context.Background(), c, &node, &v1.Node{}) + Expect(updated).To(Equal(expectedUpdate)) Expect(my_ec).To(Equal(expectedExitCode)) if rrCId != "" { Expect(node.Spec.BGP).NotTo(BeNil()) } }, - Entry("startup should terminate if IP is set to none and Calico is used for networking", "bird", 1, ""), - Entry("startup should NOT terminate if IP is set to none and Calico is policy-only", "none", 0, ""), - Entry("startup should NOT terminate and BGPSpec shouldn't be set to nil", "none", 0, "rrClusterID"), + Entry("startup should terminate if IP is set to none and Calico is used for networking", "bird", 1, "", false), + Entry("startup should NOT terminate if IP is set to none and Calico is policy-only", "none", 0, "", false), + Entry("startup should NOT terminate and BGPSpec shouldn't be set to nil", "none", 0, "rrClusterID", true), ) var _ = Describe("Default IPv4 pool CIDR", func() { @@ -876,7 +877,7 @@ var _ = Describe("FV tests against a real etcd", func() { os.Setenv(env.key, env.value) } - configureNodeRef(node) + Expect(configureNodeRef(node)).To(Equal(true)) // If we receive an invalid env var then none will be set. if len(node.Spec.OrchRefs) > 0 { ref := node.Spec.OrchRefs[0] @@ -893,7 +894,7 @@ var _ = Describe("FV tests against a real etcd", func() { os.Setenv("CALICO_UNKNOWN_NODE_REF", "node1") node := &libapi.Node{} - configureNodeRef(node) + Expect(configureNodeRef(node)).To(Equal(false)) Expect(node.Spec.OrchRefs).To(HaveLen(0)) }) @@ -902,7 +903,7 @@ var _ = Describe("FV tests against a real etcd", func() { node := &libapi.Node{} node.Spec.OrchRefs = append(node.Spec.OrchRefs, libapi.OrchRef{"node1", "k8s"}) // nolint: vet - configureNodeRef(node) + Expect(configureNodeRef(node)).To(Equal(true)) Expect(node.Spec.OrchRefs).To(HaveLen(1)) })