Skip to content

Commit

Permalink
Use GetNodes in metrics-helper; explicitly install latest addon (#2093)
Browse files Browse the repository at this point in the history
* Use GetNodes in metrics-helper; explicitly install latest addon

* make format changes to pass make check-format

* update cni image default to latest image in repo

Co-authored-by: Jayanth Varavani <1111446+jayanthvn@users.noreply.github.com>
  • Loading branch information
jdn5126 and jayanthvn authored Oct 11, 2022
1 parent b4f1777 commit 9023f57
Show file tree
Hide file tree
Showing 18 changed files with 51 additions and 40 deletions.
4 changes: 2 additions & 2 deletions cmd/cni-metrics-helper/metrics/pod_watcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ func NewDefaultPodWatcher(k8sClient client.Client, log logger.Logger) *defaultPo
}
}

//Returns aws-node pod info. Below function assumes CNI pods follow aws-node* naming format
//and so the function has to be updated if the CNI pod name format changes.
// Returns aws-node pod info. Below function assumes CNI pods follow aws-node* naming format
// and so the function has to be updated if the CNI pod name format changes.
func (d *defaultPodWatcher) GetCNIPods(ctx context.Context) ([]string, error) {
var CNIPods []string
var podList corev1.PodList
Expand Down
2 changes: 1 addition & 1 deletion cmd/routed-eni-cni-plugin/cni.go
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ func del(args *skel.CmdArgs, cniTypes typeswrapper.CNITYPES, grpcClient grpcwrap
log.Debugf("Prev Result: %v\n", conf.PrevResult)

if err != nil {
return errors.Wrap(err, "add cmd: error loading config from args")
return errors.Wrap(err, "del cmd: error loading config from args")
}

log.Infof("Received CNI del request: ContainerID(%s) Netns(%s) IfName(%s) Args(%s) Path(%s) argsStdinData(%s)",
Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/crd/v1alpha1/groupversion_info.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ limitations under the License.
*/

// Package v1alpha1 contains API Schema definitions for the crd v1alpha1 API group
//+kubebuilder:object:generate=true
//+groupName=crd.example.com
// +kubebuilder:object:generate=true
// +groupName=crd.example.com
package v1alpha1

import (
Expand Down
14 changes: 7 additions & 7 deletions pkg/awsutils/awsutils.go
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ func prometheusRegister() {
}
}

//StringSet is a set of strings
// StringSet is a set of strings
type StringSet struct {
sync.RWMutex
data sets.String
Expand Down Expand Up @@ -780,7 +780,7 @@ func (cache *EC2InstanceMetadataCache) AllocENI(useCustomCfg bool, sg []*string,
return eniID, nil
}

// attachENI calls EC2 API to attach the ENI and returns the attachment id
// attachENI calls EC2 API to attach the ENI and returns the attachment id
func (cache *EC2InstanceMetadataCache) attachENI(eniID string) (string, error) {
// attach to instance
freeDevice, err := cache.awsGetFreeDeviceNumber()
Expand Down Expand Up @@ -1829,7 +1829,7 @@ func (cache *EC2InstanceMetadataCache) GetPrimaryENImac() string {
return cache.primaryENImac
}

//SetUnmanagedENIs Set unmanaged ENI set
// SetUnmanagedENIs Set unmanaged ENI set
func (cache *EC2InstanceMetadataCache) SetUnmanagedENIs(eniIDs []string) {
cache.unmanagedENIs.Set(eniIDs)
}
Expand All @@ -1839,7 +1839,7 @@ func (cache *EC2InstanceMetadataCache) GetInstanceID() string {
return cache.instanceID
}

//IsUnmanagedENI returns if the eni is unmanaged
// IsUnmanagedENI returns if the eni is unmanaged
func (cache *EC2InstanceMetadataCache) IsUnmanagedENI(eniID string) bool {
if len(eniID) != 0 {
return cache.unmanagedENIs.Has(eniID)
Expand Down Expand Up @@ -1872,23 +1872,23 @@ func (cache *EC2InstanceMetadataCache) getENIsFromPaginatedDescribeNetworkInterf
return innerErr
}

//SetCNIUnmanagedENIs Set unmanaged ENI set
// SetCNIUnmanagedENIs Set unmanaged ENI set
func (cache *EC2InstanceMetadataCache) SetCNIUnmanagedENIs(eniID []string) error {
if len(eniID) != 0 {
cache.cniunmanagedENIs.Set(eniID)
}
return nil
}

//IsCNIUnmanagedENI returns if the eni is unmanaged
// IsCNIUnmanagedENI returns if the eni is unmanaged
func (cache *EC2InstanceMetadataCache) IsCNIUnmanagedENI(eniID string) bool {
if len(eniID) != 0 {
return cache.cniunmanagedENIs.Has(eniID)
}
return false
}

//IsPrimaryENI returns if the eni is unmanaged
// IsPrimaryENI returns if the eni is unmanaged
func (cache *EC2InstanceMetadataCache) IsPrimaryENI(eniID string) bool {
if len(eniID) != 0 && eniID == cache.GetPrimaryENI() {
return true
Expand Down
2 changes: 1 addition & 1 deletion pkg/cri/cri.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ func New() *Client {
return &Client{}
}

//GetRunningPodSandboxes get running sandboxIDs
// GetRunningPodSandboxes get running sandboxIDs
func (c *Client) GetRunningPodSandboxes(log logger.Logger) ([]SandboxInfo, error) {
ctx := context.TODO()

Expand Down
4 changes: 2 additions & 2 deletions pkg/ec2wrapper/ec2wrapper.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//Package ec2wrapper is used to wrap around the ec2 service APIs
// Package ec2wrapper is used to wrap around the ec2 service APIs
package ec2wrapper

import (
Expand Down Expand Up @@ -27,7 +27,7 @@ type EC2Wrapper struct {
instanceIdentityDocument ec2metadata.EC2InstanceIdentityDocument
}

//NewMetricsClient returns an instance of the EC2 wrapper
// NewMetricsClient returns an instance of the EC2 wrapper
func NewMetricsClient() (*EC2Wrapper, error) {
sess := awssession.New()
ec2MetadataClient := ec2metadatawrapper.New(sess)
Expand Down
2 changes: 1 addition & 1 deletion pkg/eniconfig/eniconfig_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
// not use this file except in compliance with the License. A copy of the
// License is located at
//
// http://aws.amazon.com/apache2.0/
// http://aws.amazon.com/apache2.0/
//
// or in the "license" file accompanying this file. This file is distributed
// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
Expand Down
9 changes: 5 additions & 4 deletions pkg/ipamd/datastore/data_store.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,9 @@ var (
)

// IPAMKey is the IPAM primary key. Quoting CNI spec:
// Plugins that store state should do so using a primary key of
// (network name, CNI_CONTAINERID, CNI_IFNAME).
//
// Plugins that store state should do so using a primary key of
// (network name, CNI_CONTAINERID, CNI_IFNAME).
type IPAMKey struct {
NetworkName string `json:"networkName"`
ContainerID string `json:"containerID"`
Expand Down Expand Up @@ -232,7 +233,7 @@ func (e *ENI) AssignedIPv4Addresses() int {
return count
}

//AssignedIPAddressesInCidr is the number of IP addresses already assigned in the IPv4 CIDR
// AssignedIPAddressesInCidr is the number of IP addresses already assigned in the IPv4 CIDR
func (cidr *CidrInfo) AssignedIPAddressesInCidr() int {
count := 0
//SIP : This will run just once and count will be 0 if addr is not assigned or addr is not allocated yet(unused IP)
Expand Down Expand Up @@ -1467,7 +1468,7 @@ func getNextIPAddr(ip net.IP) {
}
}

//Function to return PD defaults supported by VPC
// Function to return PD defaults supported by VPC
func GetPrefixDelegationDefaults() (int, int, int) {
numPrefixesPerENI := 1
numIPsPerPrefix := 16
Expand Down
2 changes: 1 addition & 1 deletion pkg/ipamd/ipamd.go
Original file line number Diff line number Diff line change
Expand Up @@ -2122,7 +2122,7 @@ func (c *IPAMContext) warmPrefixTargetDefined() bool {
return c.warmPrefixTarget >= defaultWarmPrefixTarget && c.enablePrefixDelegation
}

//DeallocCidrs frees IPs and Prefixes from EC2
// DeallocCidrs frees IPs and Prefixes from EC2
func (c *IPAMContext) DeallocCidrs(eniID string, deletableCidrs []datastore.CidrInfo) {
var deletableIPs []string
var deletablePrefixes []string
Expand Down
2 changes: 1 addition & 1 deletion pkg/networkutils/network.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ func (n *linuxNetwork) SetupRuleToBlockNodeLocalV4Access() error {
return n.setupRuleToBlockNodeLocalV4Access()
}

//Setup a rule to block traffic directed to v4 interface of the Pod
// Setup a rule to block traffic directed to v4 interface of the Pod
func (n *linuxNetwork) setupRuleToBlockNodeLocalV4Access() error {
ipt, err := n.newIptables(iptables.ProtocolIPv4)
if err != nil {
Expand Down
8 changes: 4 additions & 4 deletions pkg/utils/logger/logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,13 @@
// Package logger is the CNI Logger interface, using zap
package logger

//Log is global variable so that log functions can be directly accessed
// Log is global variable so that log functions can be directly accessed
var log Logger

//Fields Type to pass when we want to call WithFields for structured logging
// Fields Type to pass when we want to call WithFields for structured logging
type Fields map[string]interface{}

//Logger is our contract for the logger
// Logger is our contract for the logger
type Logger interface {
Debugf(format string, args ...interface{})

Expand Down Expand Up @@ -55,7 +55,7 @@ func Get() Logger {
return log
}

//New logger initializes logger
// New logger initializes logger
func New(inputLogConfig *Configuration) Logger {
log = inputLogConfig.newZapLogger()
log.Info("Constructed new logger instance")
Expand Down
2 changes: 1 addition & 1 deletion pkg/utils/logger/zaplogger.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ func getPluginLogFilePath(logFilePath string) zapcore.WriteSyncer {
return writer
}

//getLogWriter is for lumberjack
// getLogWriter is for lumberjack
func getLogWriter(logFilePath string) zapcore.WriteSyncer {
lumberJackLogger := &lumberjack.Logger{
Filename: logFilePath,
Expand Down
3 changes: 3 additions & 0 deletions scripts/run-cni-release-tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,12 @@ if [[ -n "${ENDPOINT}" ]]; then
ENDPOINT_FLAG="--endpoint $ENDPOINT"
fi


echo "Running release tests on cluster: $CLUSTER_NAME in region: $REGION"

load_cluster_details
START=$SECONDS
run_integration_test
run_calico_tests

echo "Completed running all tests in $((SECONDS - START)) seconds."
5 changes: 4 additions & 1 deletion scripts/run-ipv6-canary-test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ set -e
SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"
GINKGO_TEST_BUILD="$SCRIPT_DIR/../test/build"

source "$SCRIPT_DIR"/lib/add-on.sh
source "$SCRIPT_DIR"/lib/cluster.sh
source "$SCRIPT_DIR"/lib/canary.sh

Expand All @@ -17,9 +18,11 @@ function run_ginkgo_test() {
}

load_cluster_details
load_addon_details

echo "Running IPv6 Canary tests"
echo "Running IPv6 Canary tests on the latest addon version"

install_add_on "$LATEST_ADDON_VERSION"
run_ginkgo_test "CANARY"

echo "all tests ran successfully in $(($SECONDS / 60)) minutes and $(($SECONDS % 60)) seconds"
23 changes: 14 additions & 9 deletions scripts/update-cni-images.sh
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,21 @@ source "$SCRIPTS_DIR/lib/k8s.sh"
AWS_K8S_CNI_MANIFEST="$SCRIPTS_DIR/../config/master/aws-k8s-cni.yaml"
MANIFEST_IMG_VERSION=`grep "image:" $AWS_K8S_CNI_MANIFEST | cut -d ":" -f3 | cut -d "\"" -f1 | head -1`

# Replace the images in aws-k8s-cni.yaml with the tester images
echo "Replacing images in aws-k8s-cni manifest with $AMAZON_K8S_CNI and $AMAZON_K8S_CNI_INIT"
sed -i'.bak' "s,602401143452.dkr.ecr.us-west-2.amazonaws.com/amazon-k8s-cni:$MANIFEST_IMG_VERSION,$AMAZON_K8S_CNI," "$AWS_K8S_CNI_MANIFEST"
sed -i'.bak' "s,602401143452.dkr.ecr.us-west-2.amazonaws.com/amazon-k8s-cni-init:$MANIFEST_IMG_VERSION,$AMAZON_K8S_CNI_INIT," "$AWS_K8S_CNI_MANIFEST"

# grep to verify replacement
grep -r -q $AMAZON_K8S_CNI $AWS_K8S_CNI_MANIFEST
grep -r -q $AMAZON_K8S_CNI_INIT $AWS_K8S_CNI_MANIFEST
# Replace the images in aws-k8s-cni.yaml with the tester images when environment variables are set
if [[ -z $AWS_K8S_CNI ]]; then
echo "Applying latest CNI image from aws-k8s-cni manifest"
else
echo "Replacing CNI image in aws-k8s-cni manifest with $AMAZON_K8S_CNI"
sed -i'.bak' "s,602401143452.dkr.ecr.us-west-2.amazonaws.com/amazon-k8s-cni:$MANIFEST_IMG_VERSION,$AMAZON_K8S_CNI," "$AWS_K8S_CNI_MANIFEST"
fi
if [[ -z $AWS_K8S_CNI_INIT ]]; then
echo "Applying latest CNI init image from aws-k8s-cni manifest"
else
echo "Replacing CNI image in aws-k8s-cni manifest with $AMAZON_K8S_CNI"
sed -i'.bak' "s,602401143452.dkr.ecr.us-west-2.amazonaws.com/amazon-k8s-cni-init:$MANIFEST_IMG_VERSION,$AMAZON_K8S_CNI_INIT," "$AWS_K8S_CNI_MANIFEST"
fi

echo "Applying aws-k8s-cni.yaml manifest to aws-node daemonset"
kubectl apply -f $AWS_K8S_CNI_MANIFEST

check_ds_rollout "aws-node" "kube-system" "4m"
check_ds_rollout "aws-node" "kube-system" "4m"
2 changes: 1 addition & 1 deletion test/e2e/snat/snat_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ var _ = BeforeSuite(func() {
CreateNamespace(testUtils.DefaultTestNamespace)

By("Getting existing nodes in the cluster")
nodes, err := f.K8sResourceManagers.NodeManager().GetAllNodes()
nodes, err := f.K8sResourceManagers.NodeManager().GetNodes(f.Options.NgNameLabelKey, f.Options.NgNameLabelVal)
Expect(err).ToNot(HaveOccurred())

By("verifying more than 1 nodes are present for the test")
Expand Down
1 change: 0 additions & 1 deletion test/framework/resources/k8s/resources/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ func (d defaultDeploymentManager) CreateAndWaitTillDeploymentIsReady(deployment
return d.WaitTillDeploymentReady(deployment, timeout)
}

//
func (d defaultDeploymentManager) DeleteAndWaitTillDeploymentIsDeleted(deployment *v1.Deployment) error {
ctx := context.Background()
err := d.k8sClient.Delete(ctx, deployment)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ var _ = BeforeSuite(func() {
CreateNamespace(utils.DefaultTestNamespace)

By("getting the node list")
nodeList, err := f.K8sResourceManagers.NodeManager().GetAllNodes()
nodeList, err := f.K8sResourceManagers.NodeManager().GetNodes(f.Options.NgNameLabelKey, f.Options.NgNameLabelVal)
Expect(err).ToNot(HaveOccurred())
Expect(len(nodeList.Items)).To(BeNumerically(">", 0))

Expand Down

0 comments on commit 9023f57

Please sign in to comment.