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

[gaorong]

Now we can talk to apiserver by specifying --kubeconfig or --master flag other than default in-cluster config, so we can remove the dependence on kube-proxy, that is to say, aws-k8s-agent can start successfully without kube-proxy.

Have verified in our aws environment and everything works fine.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[jaypipes]

A few things I'd like to see changed... please see inline comments.

Thanks!
-jay

[jaypipes]

can we call this variable apiServerAddr please instead of master?

[jaypipes]

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

[jaypipes]

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

[jaypipes]

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.

[gaorong]

@jaypipes I would change as you advise, but there are some other questions need to discuss.

I found it more difficult than I imagined. For now, the amazon-k8s-cni docker image has a hardcode ENTRYPOINT /app/install-aws.sh. When we need to add the starting flags to aws-k8s-agent, we must modify this shell script then rebuild the docker image, which seems unacceptable.

we need to

1. change ENTRYPOINT /app/install-aws.sh to CMD /app/install-aws.sh, so that we can change the starting command and specify various flags without the need of rebuilding image.
2. remove the execution of /app/aws-k8s-agent from /app/install-aws.sh, so that we can add flages to aws-k8s-agent and run this daemon alone without the need of changing this script file.

Based on this precondition, there are two workflows we can follow:

Option 1

• add a init container to execute /app/install-aws.sh. In the install-aws.sh shell script, we only do some preparation work and do not execute exec /app/aws-k8s-agent
• add a main container, whose command is: /app/aws-k8s-agent --kubeconfig /kubeconfig/config.

The YAML of pod looks like this:

...
      initContainers:
      - command:
        - sh
        - -c
        - /app/install-aws.sh
        image: 602401143452.dkr.ecr.us-west-2.amazonaws.com/amazon-k8s-cni
        volumeMounts:
        - mountPath: /host/opt/cni/bin
          name: cni-bin-dir
        - mountPath: /host/etc/cni/net.d
          name: cni-net-dir
        - mountPath: /host/var/log
          name: log-dir
      containers:
        image: 602401143452.dkr.ecr.us-west-2.amazonaws.com/amazon-k8s-cni
        command:
        - /app/aws-k8s-agent
        - -kubeconfig
        - /kubeconfig/config
        imagePullPolicy: IfNotPresent
        name: aws-node
...

I would like this way, this pattern is more clear.
this is also the way of alibabaCloud's cni: https://github.com/AliyunContainerService/terway/blob/master/terway-multiip.yml#L110-L137

but the question is: now we have some hardcode paths in our main.go:

amazon-vpc-cni-k8s/main.go

Lines 78 to 90 in 3b4fd50

	log.Info("Copying /app/aws-cni to /host/opt/cni/bin/aws-cni")
	err = copyFileContents("/app/aws-cni", "/host/opt/cni/bin/aws-cni")
	if err != nil {
	log.Errorf("Failed to copy aws-cni: %v", err)
	return 1
	}
	log.Info("Copying /app/10-aws.conflist to /host/etc/cni/net.d/10-aws.conflist")
	err = copyFileContents("/app/10-aws.conflist", "/host/etc/cni/net.d/10-aws.conflist")
	if err != nil {
	log.Errorf("Failed to copy 10-aws.conflist: %v", err)
	return 1
	}

all those paths are prepared by /app/install-aws.sh in init container and then used by the main container, however, the init container and main container have different rootfs and can not share files. the main container would not found all these files and failed to run. we need to add a HostPath volume to let those two container transfer files.

Option 2

• change the main container's command to: sh -c "/app/install-aws.sh && /app/aws-k8s-agent - -kubeconfig /kubeconfig/config".

this option would be easier to achieve.

Any thought?

[jaypipes]

@jaypipes I would change as you advise, but there are some other questions need to discuss.

I found it more difficult than I imagined. For now, the amazon-k8s-cni docker image has a hardcode ENTRYPOINT /app/install-aws.sh. When we need to add the starting flags to aws-k8s-agent, we must modify this shell script then rebuild the docker image, which seems unacceptable.

I went with your first suggestion, which I thought was excellent:

#727

[jaypipes]

Hi @gaorong! Hey, we've gone ahead and reworked the entrypoint script entirely (see #735) so you should be OK to rebase this and address the issues I raised in the initial review. Let me know if you need any assistance or have any questions!

[gaorong]

@jaypipes Sorry for the late reply.

We have bypassed this issue.

Besides, My account of the aws cluster has run out of crash and been destroyed, I’m sorry that I can not continue to work on this.

[jaypipes]

No problem @gaorong. I'll pick up your work on this and drive to completion. Thanks!