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.

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

support pass in kubeconfig and master to client-go by flag #706

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion cni-metrics-helper/cni-metrics-helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ func main() {
flags.Lookup("logtostderr").DefValue = "true"
flags.Lookup("logtostderr").NoOptDefVal = "true"
flags.BoolVar(&options.submitCW, "cloudwatch", true, "a bool")
flags.StringVar(&options.kubeconfig, "kubeconfig", "", "Path to a kubeconfig file, specifying how to connect to the API server.")

flags.Usage = func() {
_, _ = fmt.Fprintf(os.Stderr, "Usage of %s:\n", os.Args[0])
Expand Down Expand Up @@ -79,7 +80,7 @@ func main() {

glog.Infof("Starting CNIMetricsHelper. Sending metrics to CloudWatch: %v", options.submitCW)

kubeClient, err := k8sapi.CreateKubeClient()
kubeClient, err := k8sapi.CreateKubeClient(options.kubeconfig, "")
if err != nil {
glog.Errorf("Failed to create client: %v", err)
os.Exit(1)
Expand Down
13 changes: 11 additions & 2 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
package main

import (
"flag"
"io"
"os"

Expand All @@ -31,20 +32,28 @@ const (
)

var (
version string
version string
kubeconfig string
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we call this variable kubeconfigPath please, to indicate it is a filepath not a config object?

master string
Copy link
Contributor

Choose a reason for hiding this comment

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

can we call this variable apiServerAddr please instead of master?

)

func init() {
flag.StringVar(&master, "master", "", "The address of the Kubernetes API server (overrides any value in kubeconfig).")
flag.StringVar(&kubeconfig, "kubeconfig", "", "Path to kubeconfig file with authorization and master location information.")
}

func main() {
os.Exit(_main())
}

func _main() int {
defer log.Flush()
flag.Parse()
logger.SetupLogger(logger.GetLogFileLocation(defaultLogFilePath))

log.Infof("Starting L-IPAMD %s ...", version)

kubeClient, err := k8sapi.CreateKubeClient()
kubeClient, err := k8sapi.CreateKubeClient(kubeconfig, master)
if err != nil {
log.Errorf("Failed to create client: %v", err)
return 1
Expand Down
20 changes: 17 additions & 3 deletions pkg/k8sapi/discovery.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,20 @@ import (
"sync"
"time"

"github.com/operator-framework/operator-sdk/pkg/k8sclient"
"github.com/pkg/errors"

log "github.com/cihub/seelog"

clientset "k8s.io/client-go/kubernetes"

"github.com/operator-framework/operator-sdk/pkg/k8sclient"
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/fields"
"k8s.io/apimachinery/pkg/util/runtime"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/client-go/kubernetes"
"k8s.io/client-go/tools/cache"
"k8s.io/client-go/tools/clientcmd"
"k8s.io/client-go/util/workqueue"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -80,8 +81,21 @@ func NewController(clientset kubernetes.Interface) *Controller {
}

// CreateKubeClient creates a k8s client
func CreateKubeClient() (clientset.Interface, error) {
kubeClient := k8sclient.GetKubeClient()
func CreateKubeClient(kubeconfig, master string) (clientset.Interface, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is called from cni-metrics-helper, and is the reason your unit tests are failing:

# github.com/aws/amazon-vpc-cni-k8s/cni-metrics-helper
cni-metrics-helper/cni-metrics-helper.go:82:44: not enough arguments in call to k8sapi.CreateKubeClient

var kubeClient clientset.Interface
if master == "" && kubeconfig == "" {
kubeClient = k8sclient.GetKubeClient()
} else {
config, err := clientcmd.BuildConfigFromFlags(master, kubeconfig)
Copy link
Contributor

Choose a reason for hiding this comment

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

Frankly, client-go/tools/clientcmd.BuildConfigFromFlags is able to accept empty kube API server URL and config path:

https://github.com/kubernetes/kubernetes/blob/1c974109b672ade9aae86f2daf1d50c89c36949f/staging/src/k8s.io/client-go/tools/clientcmd/client_config.go#L544

and if it does, loads the inClusterConfig appropriately, which is what the k8sclient.GetKubeClient call does. So, I would rather just remove the whole operator-sdk k8sclient pkg dependency here and just use client-go's clientcmd for this.

The dependency amazon-vpc-cni-k8s has on operator-sdk is ancient (0.0.7 which was released >1 year ago) and modern releases of operator-sdk don't even have a k8sclient package at all.

if err != nil {
panic(err)
}
kubeClient, err = clientset.NewForConfig(config)
if err != nil {
panic(err)
}
}

// Informers don't seem to do a good job logging error messages when it
// can't reach the server, making debugging hard. This makes it easier to
// figure out if apiserver is configured incorrectly.
Expand Down