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

Use dockershim.sock for CRI #751

Merged
merged 1 commit into from
Dec 11, 2019
Merged

Use dockershim.sock for CRI #751

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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion config/v1.5/aws-k8s-cni.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ spec:
- name: AWS_VPC_K8S_CNI_VETHPREFIX
value: eni
- name: AWS_VPC_K8S_CNI_MTU
value: 9001
value: "9001"
Copy link
Contributor

Choose a reason for hiding this comment

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

interesting. this was a problem?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, for some reason I got an error applying the config without the quotes.

- name: MY_NODE_NAME
valueFrom:
fieldRef:
Expand All @@ -120,6 +120,8 @@ spec:
name: log-dir
- mountPath: /var/run/docker.sock
name: dockersock
- mountPath: /var/run/dockershim.sock
name: dockershim
volumes:
- name: cni-bin-dir
hostPath:
Expand All @@ -133,6 +135,9 @@ spec:
- name: dockersock
hostPath:
path: /var/run/docker.sock
- name: dockershim
hostPath:
path: /var/run/dockershim.sock

---
apiVersion: apiextensions.k8s.io/v1beta1
Expand Down
5 changes: 2 additions & 3 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ require (
github.com/json-iterator/go v1.1.5 // indirect
github.com/matttproud/golang_protobuf_extensions v1.0.1 // indirect
github.com/modern-go/reflect2 v0.0.0-20180701023420-4b7aa43c6742 // indirect
github.com/onsi/ginkgo v1.8.0
github.com/onsi/gomega v1.5.0
github.com/onsi/ginkgo v1.8.0 // indirect
github.com/onsi/gomega v1.5.0 // indirect
github.com/operator-framework/operator-sdk v0.0.7
github.com/peterbourgon/diskv v2.0.1+incompatible // indirect
github.com/pkg/errors v0.8.1
Expand All @@ -40,7 +40,6 @@ require (
golang.org/x/net v0.0.0-20191004110552-13f9640d40b9
golang.org/x/sys v0.0.0-20190826190057-c7b8b68b1456
golang.org/x/time v0.0.0-20180412165947-fbb02b2291d2 // indirect
google.golang.org/appengine v1.4.0 // indirect
Copy link
Contributor

Choose a reason for hiding this comment

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

hmm. this is likely due to different Golang versions in our local development environments trampling on each other... My local go version is 1.12.7. I suspect you're on 1.13 and beyond?

google.golang.org/grpc v1.23.1
gopkg.in/inf.v0 v0.9.1 // indirect
k8s.io/api v0.0.0-20180712090710-2d6f90ab1293
Expand Down
2 changes: 1 addition & 1 deletion ipamd/datastore/data_store.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -470,7 +470,7 @@ func (ds *DataStore) UnassignPodIPv4Address(k8sPod *k8sapi.K8SPodInfo) (ip strin
}
ipAddr, ok := ds.podsIP[podKey]
if !ok {
log.Warnf("UnassignPodIPv4Address: Failed to find pod %s namespace %s Sandbox %s",
log.Warnf("UnassignPodIPv4Address: Failed to find pod %s namespace %q, sandbox %q",
k8sPod.Name, k8sPod.Namespace, k8sPod.Sandbox)
return "", 0, ErrUnknownPod
}
Expand Down
2 changes: 1 addition & 1 deletion ipamd/ipamd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -334,14 +334,14 @@ func (c *IPAMContext) nodeInit() error {
}
}
localPods, err := c.getLocalPodsWithRetry()
log.Debugf("getLocalPodsWithRetry() found %d local pods", len(localPods))
if err != nil {
log.Warnf("During ipamd init, failed to get Pod information from Kubernetes API Server %v", err)
ipamdErrInc("nodeInitK8SGetLocalPodIPsFailed")
// This can happens when L-IPAMD starts before kubelet.
// TODO need to add node health stats here
return errors.Wrap(err, "failed to get running pods!")
}
log.Debugf("getLocalPodsWithRetry() found %d local pods", len(localPods))
Copy link
Contributor

Choose a reason for hiding this comment

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

good move...


rules, err := c.networkClient.GetRuleList()
if err != nil {
Expand Down
10 changes: 8 additions & 2 deletions pkg/cri/cri.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,16 @@ package cri
import (
"context"
"errors"

"google.golang.org/grpc"
runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1alpha2"
"os"

log "github.com/cihub/seelog"
)

const (
criSocketPath = "unix:///var/run/cri.sock"
dockerSocketPath = "unix:///var/run/dockershim.sock"
)

// SandboxInfo provides container information
Expand All @@ -33,7 +34,12 @@ func New() *Client {
}

func (c *Client) GetRunningPodSandboxes() (map[string]*SandboxInfo, error) {
conn, err := grpc.Dial(criSocketPath, grpc.WithInsecure())
socketPath := dockerSocketPath
if info, err := os.Stat("/var/run/cri.sock"); err == nil && !info.IsDir() {
socketPath = criSocketPath
}
log.Debugf("Getting running pod sandboxes from %q", socketPath)
conn, err := grpc.Dial(socketPath, grpc.WithInsecure())
Copy link
Contributor

Choose a reason for hiding this comment

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

I suppose we can turn this into a blocking call (WithBlock() with a backoff timeout) in a future commit...

if err != nil {
return nil, err
}
Expand Down
16 changes: 8 additions & 8 deletions scripts/entrypoint.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ fi

AGENT_LOG_PATH=${AGENT_LOG_PATH:-aws-k8s-agent.log}
HOST_CNI_BIN_PATH=${HOST_CNI_BIN_PATH:-/host/opt/cni/bin}
HOST_CNI_CONFDIR_PATH=${HOST_CNI_CONFDIR_PATH:-/host/opt/cni/net.d}
HOST_CNI_CONFDIR_PATH=${HOST_CNI_CONFDIR_PATH:-/host/etc/cni/net.d}
Copy link
Contributor

Choose a reason for hiding this comment

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

gahhhhhh!


# Checks for IPAM connectivity on localhost port 50051, retrying connectivity
# check with a timeout of 36 seconds
Expand All @@ -53,7 +53,7 @@ wait_for_ipam() {
}

echo -n "starting IPAM daemon in background ... "
./aws-k8s-agent > $AGENT_LOG_PATH 2>&1 &
./aws-k8s-agent > "$AGENT_LOG_PATH" 2>&1 &
echo "ok."

echo -n "checking for IPAM connectivity ... "
Expand All @@ -68,13 +68,13 @@ echo "ok."

echo -n "copying CNI plugin binaries and config files ... "

cp portmap $HOST_CNI_BIN_PATH
cp aws-cni $HOST_CNI_BIN_PATH$
cp aws-cni-support.sh $HOST_CNI_BIN_PATH
cp portmap "$HOST_CNI_BIN_PATH"
cp aws-cni "$HOST_CNI_BIN_PATH"
cp aws-cni-support.sh "$HOST_CNI_BIN_PATH"

sed -i s/__VETHPREFIX__/"${AWS_VPC_K8S_CNI_VETHPREFIX:-"eni"}"/g 10-aws.conflist
sed -i s/__MTU__/"${AWS_VPC_ENI_MTU:-"9001"}"/g 10-aws.conflist
cp 10-aws.conflist $HOST_CNI_CONFDIR_PATH
cp 10-aws.conflist "$HOST_CNI_CONFDIR_PATH"

echo " ok."

Expand All @@ -84,8 +84,8 @@ fi

# bring the aws-k8s-agent process back into the foreground
echo "foregrounding IPAM daemon ... "
fg %1 >/dev/null 2>&1 || $(echo "failed (process terminated)" && cat $AGENT_LOG_PATH && exit 1)
fg %1 >/dev/null 2>&1 || $(echo "failed (process terminated)" && cat "$AGENT_LOG_PATH" && exit 1)

# Best practice states we should send the container's CMD output to stdout, so
# let's tee back up the log file into stdout
cat $AGENT_LOG_PATH
cat "$AGENT_LOG_PATH"
Copy link
Contributor

Choose a reason for hiding this comment

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

interesting that quote-encapping the variable in the entrypoint.sh script fixed anything here and above. I know it's safer to quote-encap vars but I'm curious whether this was an actual fix for a problem in the script.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It didn't, but we were mixing quoted and unquoted, so I just made it consistent.