Skip to content

Update cluster-api release-0.1 vendor #750

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

Merged
k8s-ci-robot merged 1 commit into from
May 2, 2019
Merged

Update cluster-api release-0.1 vendor #750

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
12 changes: 10 additions & 2 deletions Gopkg.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions cmd/clusterctl/examples/aws/generate-yaml.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ MACHINES_TEMPLATE_FILE=${DIR}/machines.yaml.template
MACHINES_GENERATED_FILE=${OUTPUT_DIR}/machines.yaml
HA_MACHINES_TEMPLATE_FILE=${DIR}/machines-ha.yaml.template
HA_MACHINES_GENERATED_FILE=${OUTPUT_DIR}/machines-ha.yaml
DEPLOYMENT_MACHINES_TEMPLATE_FILE=${DIR}/machine-deployment.yaml.template
DEPLOYMENT_MACHINES_GENERATED_FILE=${OUTPUT_DIR}/machine-deployment.yaml
ADDONS_FILE=${OUTPUT_DIR}/addons.yaml
PROVIDER_COMPONENTS_SRC=${DIR}/provider-components-base.yaml
PROVIDER_COMPONENTS_FILE=${OUTPUT_DIR}/provider-components.yaml
Expand Down Expand Up @@ -106,6 +108,9 @@ echo "Done generating ${MACHINES_GENERATED_FILE}"
$ENVSUBST < $HA_MACHINES_TEMPLATE_FILE > "${HA_MACHINES_GENERATED_FILE}"
echo "Done generating ${HA_MACHINES_GENERATED_FILE}"

$ENVSUBST < $DEPLOYMENT_MACHINES_TEMPLATE_FILE > "${DEPLOYMENT_MACHINES_GENERATED_FILE}"
echo "Done generating ${DEPLOYMENT_MACHINES_GENERATED_FILE}"

cp ${DIR}/addons.yaml ${ADDONS_FILE}
echo "Done copying ${ADDONS_FILE}"

Expand Down
27 changes: 27 additions & 0 deletions cmd/clusterctl/examples/aws/machine-deployment.yaml.template
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
apiVersion: "cluster.k8s.io/v1alpha1"
kind: MachineDeployment
metadata:
name: sample-machinedeployment
labels:
cluster.k8s.io/cluster-name: ${CLUSTER_NAME}
spec:
replicas: 1
selector:
matchLabels:
cluster.k8s.io/cluster-name: ${CLUSTER_NAME}
set: node
template:
metadata:
labels:
cluster.k8s.io/cluster-name: ${CLUSTER_NAME}
set: node
spec:
versions:
kubelet: v1.13.5
providerSpec:
value:
apiVersion: awsprovider/v1alpha1
kind: AWSMachineProviderSpec
instanceType: "${NODE_MACHINE_TYPE}"
iamInstanceProfile: "nodes.cluster-api-provider-aws.sigs.k8s.io"
keyName: "${SSH_KEY_NAME}"
17 changes: 0 additions & 17 deletions cmd/clusterctl/examples/aws/machines-ha.yaml.template
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,20 +55,3 @@ items:
instanceType: "${CONTROL_PLANE_MACHINE_TYPE}"
iamInstanceProfile: "control-plane.cluster-api-provider-aws.sigs.k8s.io"
keyName: "${SSH_KEY_NAME}"
- apiVersion: "cluster.k8s.io/v1alpha1"
kind: Machine
metadata:
generateName: node-
labels:
cluster.k8s.io/cluster-name: ${CLUSTER_NAME}
set: node
spec:
versions:
kubelet: v1.13.5
providerSpec:
value:
apiVersion: awsprovider/v1alpha1
kind: AWSMachineProviderSpec
instanceType: "${NODE_MACHINE_TYPE}"
iamInstanceProfile: "nodes.cluster-api-provider-aws.sigs.k8s.io"
keyName: "${SSH_KEY_NAME}"
19 changes: 14 additions & 5 deletions pkg/cloud/aws/services/ec2/instances.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,10 @@ package ec2
import (
"bytes"
"compress/gzip"
"context"
"encoding/base64"
"strings"
"time"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/request"
Expand Down Expand Up @@ -437,13 +439,20 @@ func (s *Service) runInstance(role string, i *v1alpha1.Instance) (*v1alpha1.Inst
return nil, errors.Errorf("no instance returned for reservation %v", out.GoString())
}

s.scope.V(2).Info("Waiting for instance to run", "instance-id", *out.Instances[0].InstanceId)
err = s.scope.EC2.WaitUntilInstanceRunningWithContext(
aws.BackgroundContext(),
waitTimeout := 1 * time.Minute
Copy link
Copy Markdown
Contributor

@chuckha chuckha May 2, 2019

Choose a reason for hiding this comment

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

We could drop the max retries and delay time using the WaiterOptions instead of a timeout context, but overall lowering the time `WaitUntilInstance I think this is probably a good thing to do so we don't get stuck waiting for 10 minutes ever

Copy link
Copy Markdown
Member Author

@vincepri vincepri May 2, 2019

Choose a reason for hiding this comment

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

Aren't WaiterOptions applied across every aws sdk call though? I mentioned above I'm open to actually remove this call entirely, we can rely on reconciliation to go and describe the state in a later reconciliation

Copy link
Copy Markdown
Contributor

@detiber detiber May 2, 2019

Choose a reason for hiding this comment

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

iirc, we originally added this because we had hit issues with returning without waiting previously. I want to say it might have been related to how we check for instance availability during Exists.

Copy link
Copy Markdown
Member Author

@vincepri vincepri May 2, 2019

Choose a reason for hiding this comment

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

I think that has been fixed by adding the "pending" checks to Exists https://github.com/kubernetes-sigs/cluster-api-provider-aws/blob/master/pkg/cloud/aws/services/ec2/instances.go#L51

Copy link
Copy Markdown
Contributor

@chuckha chuckha May 2, 2019

Choose a reason for hiding this comment

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

These don't apply across every sdk call, only for the waiter that gets created with the call.

Copy link
Copy Markdown
Member Author

@vincepri vincepri May 2, 2019

Choose a reason for hiding this comment

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

Ah ok, I was looking at something else

Copy link
Copy Markdown
Contributor

@detiber detiber May 2, 2019

Choose a reason for hiding this comment

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

Should this timeout be longer than 1 minute?

Copy link
Copy Markdown
Member Author

@vincepri vincepri May 2, 2019

Choose a reason for hiding this comment

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

I have seen that when this function actually works, 1m is more than enough to get the signal back. I'm also open to completely remove this function given that we can rely on reconciliation to take care of it

Copy link
Copy Markdown
Contributor

@detiber detiber May 2, 2019

Choose a reason for hiding this comment

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

We need to be careful not to make assumptions related to the Regions/Zones we are frequently testing in. From my experience not all AWS Regions are as responsive as us-east-* and us-west-*

s.scope.V(2).Info("Waiting for instance to be in running state", "instance-id", *out.Instances[0].InstanceId, "timeout", waitTimeout.String())
ctx, cancel := context.WithTimeout(aws.BackgroundContext(), waitTimeout)
Copy link
Copy Markdown
Contributor

@detiber detiber May 2, 2019

Choose a reason for hiding this comment

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

any particular reason to use aws.BackgroundContext() here? It looks like it just returns context.Background().

Copy link
Copy Markdown
Member Author

@vincepri vincepri May 2, 2019

Choose a reason for hiding this comment

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

Mostly because it's there, I can switch if necessary :D

defer cancel()

if err := s.scope.EC2.WaitUntilInstanceRunningWithContext(
ctx,
&ec2.DescribeInstancesInput{InstanceIds: []*string{out.Instances[0].InstanceId}},
request.WithWaiterLogger(&awslog{s.scope.Logger}),
)
return converters.SDKToInstance(out.Instances[0]), err
); err != nil {
s.scope.V(2).Info("Could not determine if Machine is running. Machine state might be unavailable until next renconciliation.")
}

return converters.SDKToInstance(out.Instances[0]), nil
}

// An internal type to satisfy aws' log interface.
Expand Down
202 changes: 202 additions & 0 deletions vendor/k8s.io/component-base/LICENSE
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

28 changes: 28 additions & 0 deletions vendor/k8s.io/component-base/cli/flag/BUILD.bazel
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading