Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Adding flexibility to CCM #93764

Merged
merged 1 commit into from Nov 9, 2020
Merged

Adding flexibility to CCM #93764

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
35 changes: 34 additions & 1 deletion cmd/cloud-controller-manager/.import-restrictions
View file
Open in desktop
@@ -1,4 +1,37 @@
rules:
- selectorRegexp: k8s[.]io/kubernetes
allowedPrefixes:
- k8s.io/kubernetes/cmd/cloud-controller-manager
- k8s.io/kubernetes/pkg/api/legacyscheme
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

was adding these back intentional or a bad rebase?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

intentional. This is indirectly imported by k8s.io/kubernetes/pkg/controller/testutil -> node_ipam_controller_test in `k8s.io/kubernetes/pkg/controller/nodeipam'.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could have a follow up PR to put the samples(like nodeIpamController) in its own directory and also split out the base/vanilla version from the sample showing usage of the extensions as mentioned above. In this way the .import-restrictions file might look better and make less confusion for cloud providers to follow.

Copy link
Member

@liggitt liggitt Nov 5, 2020 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes under staging look fine to me.

I'm having trouble understanding the current status/consumers and goals for each of the following:

  1. current cloud-controller-manager command / binary (cmd/cloud-controller-manager)
    • is this included in current releases or image manifests built/pushed? are there consumers of this today?
  2. ideal minimal example main()
    • I expected this to live in staging k8s.io/cloud-provider with no k8s.io/kubernetes dependencies
  3. an example that stitches in node_ipam to the minimal example (either by moving out node_ipam or by living in k8s.io/kubernetes)

Modifying the existing main() to add node_ipam without first extracting the minimal k8s.io/kubernetes-free one is making the path to the minimal on harder to see. Is that still the plan?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1.a) As a fast follow it might be nice to split the binary. Have a vanilla binary with none of the extensions in use which a cloud provider could copy and modify to get started. A sample which shows how to use each of the extension mechanisms. I think the vanilla should be here and the sample under a sample sub directory?

1.b) The CCM in K/K is not included in any releases but it is built to ensure it remains buildable. It is forked and modified into various other repos where the modified version is released. These changes should allow the cloud providers to just depend on libraries, no more forking needed.

2.a) +1, see the fast follow suggestion above.

2.b) +Alot, if we break the sample into its own directory, that should allow the base version to have no K8s.io/kubernetes dependencies. More importantly we can then enforce that the base version have no K8s.io/kubernetes dependencies.

  1. +1

- k8s.io/kubernetes/pkg/api/service
- k8s.io/kubernetes/pkg/api/v1/pod
- k8s.io/kubernetes/pkg/apis/apps
- k8s.io/kubernetes/pkg/apis/autoscaling
- k8s.io/kubernetes/pkg/apis/core
- k8s.io/kubernetes/pkg/apis/core/helper
- k8s.io/kubernetes/pkg/apis/core/install
- k8s.io/kubernetes/pkg/apis/core/pods
- k8s.io/kubernetes/pkg/apis/core/v1
- k8s.io/kubernetes/pkg/apis/core/v1/helper
- k8s.io/kubernetes/pkg/apis/core/validation
- k8s.io/kubernetes/pkg/apis/scheduling
- k8s.io/kubernetes/pkg/capabilities
- k8s.io/kubernetes/pkg/cluster/ports
- k8s.io/kubernetes/pkg/controller
- k8s.io/kubernetes/pkg/controller/nodeipam
- k8s.io/kubernetes/pkg/controller/nodeipam/config
- k8s.io/kubernetes/pkg/controller/nodeipam/ipam
- k8s.io/kubernetes/pkg/controller/nodeipam/ipam/cidrset
- k8s.io/kubernetes/pkg/controller/nodeipam/ipam/sync
- k8s.io/kubernetes/pkg/controller/nodeipam/ipam/test
- k8s.io/kubernetes/pkg/controller/testutil
- k8s.io/kubernetes/pkg/controller/util/node
- k8s.io/kubernetes/pkg/features
- k8s.io/kubernetes/pkg/fieldpath
- k8s.io/kubernetes/pkg/kubelet/types
- k8s.io/kubernetes/pkg/kubelet/util/format
- k8s.io/kubernetes/pkg/security/apparmor
- k8s.io/kubernetes/pkg/securitycontext
- k8s.io/kubernetes/pkg/util/hash
- k8s.io/kubernetes/pkg/util/node
- k8s.io/kubernetes/pkg/util/parsers
- k8s.io/kubernetes/pkg/util/taints
13 changes: 13 additions & 0 deletions cmd/cloud-controller-manager/BUILD
View file
Open in desktop
Expand Up @@ -18,19 +18,32 @@ go_library(
name = "go_default_library",
srcs = [
"controller-manager.go",
"nodeipamcontroller.go",
"providers.go",
],
importpath = "k8s.io/kubernetes/cmd/cloud-controller-manager",
deps = [
"//pkg/controller/nodeipam:go_default_library",
"//pkg/controller/nodeipam/config:go_default_library",
"//pkg/controller/nodeipam/ipam:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library",
"//staging/src/k8s.io/cloud-provider:go_default_library",
"//staging/src/k8s.io/cloud-provider/app:go_default_library",
"//staging/src/k8s.io/cloud-provider/app/config:go_default_library",
"//staging/src/k8s.io/cloud-provider/options:go_default_library",
"//staging/src/k8s.io/component-base/cli/flag:go_default_library",
"//staging/src/k8s.io/component-base/logs:go_default_library",
"//staging/src/k8s.io/component-base/metrics/prometheus/clientgo:go_default_library",
"//staging/src/k8s.io/component-base/metrics/prometheus/version:go_default_library",
"//staging/src/k8s.io/controller-manager/app:go_default_library",
"//staging/src/k8s.io/legacy-cloud-providers/aws:go_default_library",
"//staging/src/k8s.io/legacy-cloud-providers/azure:go_default_library",
"//staging/src/k8s.io/legacy-cloud-providers/gce:go_default_library",
"//staging/src/k8s.io/legacy-cloud-providers/openstack:go_default_library",
"//staging/src/k8s.io/legacy-cloud-providers/vsphere:go_default_library",
"//vendor/github.com/spf13/pflag:go_default_library",
"//vendor/k8s.io/klog/v2:go_default_library",
"//vendor/k8s.io/utils/net:go_default_library",
],
)

Expand Down
100 changes: 99 additions & 1 deletion cmd/cloud-controller-manager/controller-manager.go
View file
Open in desktop
Expand Up @@ -17,32 +17,130 @@ limitations under the License.
// The external controller manager is responsible for running controller loops that
// are cloud provider dependent. It uses the API to listen to new events on resources.

// This file should be written by each cloud provider.
// The current file demonstrate how other cloud provider should leverage CCM and it uses fake parameters. Please modify for your own use.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be changing the current cloud-controller-manager binary to use fake / example params... am I reading that correctly? Is this in use by anyone who would be affected by these changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current plan is to keep this file as a sample main.go in k/k. Other cloud providers will have their own main.go file in their repo(e.g.:https://github.com/kubernetes/cloud-provider-aws/blob/master/cmd/aws-cloud-controller-manager/main.go).
The current cloud-controller-manager binary (cmd/cloud-controller-manager) will only contain sample code and will not have consumers.


package main

import (
"fmt"
"math/rand"
"net/http"
"os"
"time"

"github.com/spf13/pflag"
cloudprovider "k8s.io/cloud-provider"
"k8s.io/cloud-provider/app"
cloudcontrollerconfig "k8s.io/cloud-provider/app/config"
"k8s.io/cloud-provider/options"
"k8s.io/component-base/cli/flag"
"k8s.io/component-base/logs"
_ "k8s.io/component-base/metrics/prometheus/clientgo" // load all the prometheus client-go plugins
_ "k8s.io/component-base/metrics/prometheus/version" // for version metric registration
genericcontrollermanager "k8s.io/controller-manager/app"
"k8s.io/klog/v2"
nodeipamconfig "k8s.io/kubernetes/pkg/controller/nodeipam/config"
)

const (
// cloudProviderName shows an sample of using hard coded parameter
cloudProviderName = "SampleCloudProviderName"

// defaultNodeMaskCIDRIPv4 is default mask size for IPv4 node cidr
defaultNodeMaskCIDRIPv4 = 24
// defaultNodeMaskCIDRIPv6 is default mask size for IPv6 node cidr
defaultNodeMaskCIDRIPv6 = 64
)

func main() {
rand.Seed(time.Now().UnixNano())

command := app.NewCloudControllerManagerCommand()
// cloudProviderConfigFile shows an sample of parse config file from flag option
var flagset *pflag.FlagSet = pflag.NewFlagSet("flagSet", pflag.ContinueOnError)
var cloudProviderConfigFile *string = flagset.String("cloud-provider-configfile", "", "This is the sample input for cloud provider config file")
pflag.CommandLine.ParseErrorsWhitelist.UnknownFlags = true
_ = pflag.CommandLine.Parse(os.Args[1:])

// this is an example of allow-listing specific controller loops
controllerList := []string{"cloud-node", "cloud-node-lifecycle", "service", "route"}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note to self: this is an example of allow-listing specific controller loops

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a follow up PR which split out the base/vanilla version from the sample showing usage of the extensions would be a good idea. It makes it easier on cloud providers who can just copy the base version and reference the extension sample.
If we put the samples in its own directory it would also cut down on the .import-restrictions allowing us to ensure we didn't allow any private dependencies in the base version.


s, err := options.NewCloudControllerManagerOptions()
if err != nil {
klog.Fatalf("unable to initialize command options: %v", err)
}
c, err := s.Config(controllerList, app.ControllersDisabledByDefault.List())
if err != nil {
fmt.Fprintf(os.Stderr, "%v\n", err)
os.Exit(1)
}

cloud, err := cloudprovider.InitCloudProvider(cloudProviderName, *cloudProviderConfigFile)
if err != nil {
klog.Fatalf("Cloud provider could not be initialized: %v", err)
}
if cloud == nil {
klog.Fatalf("cloud provider is nil")
}

if !cloud.HasClusterID() {
if c.ComponentConfig.KubeCloudShared.AllowUntaggedCloud {
klog.Warning("detected a cluster without a ClusterID. A ClusterID will be required in the future. Please tag your cluster to avoid any future issues")
} else {
klog.Fatalf("no ClusterID found. A ClusterID is required for the cloud provider to function properly. This check can be bypassed by setting the allow-untagged-cloud option")
}
}

// Initialize the cloud provider with a reference to the clientBuilder
cloud.Initialize(c.ClientBuilder, make(chan struct{}))
// Set the informer on the user cloud object
if informerUserCloud, ok := cloud.(cloudprovider.InformerUser); ok {
informerUserCloud.SetInformers(c.SharedInformers)
}

controllerInitializers := app.DefaultControllerInitializers(c.Complete(), cloud)

// Here is an example to remove the controller which is not needed.
// e.g. remove the cloud-node-lifecycle controller which current cloud provider does not need.
//delete(controllerInitializers, "cloud-node-lifecycle")

// Here is an example to add an controller(NodeIpamController) which will be used by cloud provider
// generate nodeipamconfig. Here is an sample code. Please pass the right parameter in your code.
// If you do not need additional controller, please ignore.
nodeipamconfig := nodeipamconfig.NodeIPAMControllerConfiguration{
ServiceCIDR: "sample",
SecondaryServiceCIDR: "sample",
NodeCIDRMaskSize: 11,
NodeCIDRMaskSizeIPv4: 11,
NodeCIDRMaskSizeIPv6: 111,
}
controllerInitializers["nodeipam"] = startNodeIpamControllerWrapper(c.Complete(), nodeipamconfig, cloud)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note to self: this is an example of what an override/extension could look like


command := app.NewCloudControllerManagerCommand(s, c, controllerInitializers)

// TODO: once we switch everything over to Cobra commands, we can go back to calling
// utilflag.InitFlags() (by removing its pflag.Parse() call). For now, we have to set the
// normalize func and add the go flag set by hand.
// Here is an sample
pflag.CommandLine.SetNormalizeFunc(flag.WordSepNormalizeFunc)
// utilflag.InitFlags()
logs.InitLogs()
defer logs.FlushLogs()

// the flags could be set before execute
command.Flags().VisitAll(func(flag *pflag.Flag) {
if flag.Name == "cloud-provider" {
flag.Value.Set("SampleCloudProviderFlagValue")
return
}
})
if err := command.Execute(); err != nil {
os.Exit(1)
}
}

func startNodeIpamControllerWrapper(ccmconfig *cloudcontrollerconfig.CompletedConfig, nodeipamconfig nodeipamconfig.NodeIPAMControllerConfiguration, cloud cloudprovider.Interface) func(ctx genericcontrollermanager.ControllerContext) (http.Handler, bool, error) {
return func(ctx genericcontrollermanager.ControllerContext) (http.Handler, bool, error) {
return startNodeIpamController(ccmconfig, nodeipamconfig, ctx, cloud)
}
}