Skip to content

Update cluster-api release-0.1 vendor#750

Merged
k8s-ci-robot merged 1 commit intokubernetes-sigs:masterfrom
vincepri:capi-master
May 2, 2019
Merged

Update cluster-api release-0.1 vendor#750
k8s-ci-robot merged 1 commit intokubernetes-sigs:masterfrom
vincepri:capi-master

Conversation

@vincepri
Copy link
Copy Markdown
Member

@vincepri vincepri commented May 1, 2019

Signed-off-by: Vince Prignano vincepri@vmware.com

What this PR does / why we need it:

  • Set the cluster-api dep to branch=master and updates the vendor folder. Upstream branch contains important bugfixes which we might need to backport later for next minor release.
  • Adds a machine deployment example in output
  • Fixes a typo in Makefile

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

Special notes for your reviewer:

Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.

Release note:

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels May 1, 2019
@k8s-ci-robot k8s-ci-robot requested review from detiber and justinsb May 1, 2019 21:47
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented May 1, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: vincepri

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 1, 2019
@vincepri vincepri force-pushed the capi-master branch 4 times, most recently from 219a094 to 0425003 Compare May 2, 2019 01:00
detiber
detiber reviewed May 2, 2019
View reviewed changes
Comment thread pkg/cloud/aws/services/ec2/instances.go Outdated
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

Comment thread Makefile Outdated
Comment thread pkg/cloud/aws/services/ec2/instances.go Outdated
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-*

detiber
detiber reviewed May 2, 2019
View reviewed changes
Comment thread Gopkg.toml Outdated
chuckha
chuckha reviewed May 2, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@chuckha chuckha left a comment

Choose a reason for hiding this comment

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

whoops, forgot to submit!

Comment thread pkg/cloud/aws/services/ec2/instances.go Outdated
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

Comment thread Gopkg.toml Outdated
@vincepri vincepri force-pushed the capi-master branch 2 times, most recently from b548219 to a490360 Compare May 2, 2019 20:11
@vincepri vincepri changed the title Set cluster-api dep to master Update cluster-api release-0.1 vendor May 2, 2019
@vincepri
Update cluster-api release-0.1 vendor
60cc408 
Signed-off-by: Vince Prignano <vincepri@vmware.com>
@detiber
Copy link
Copy Markdown
Contributor

detiber commented May 2, 2019

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 2, 2019
@k8s-ci-robot k8s-ci-robot merged commit c4636da into kubernetes-sigs:master May 2, 2019
detiber pushed a commit to detiber/cluster-api-provider-aws that referenced this pull request May 2, 2019
@vincepri @detiber
Update cluster-api release-0.1 vendor (kubernetes-sigs#750)
7412684 
Signed-off-by: Vince Prignano <vincepri@vmware.com>
k8s-ci-robot pushed a commit that referenced this pull request May 2, 2019
@detiber @k8s-ci-robot
Backport latest changes to release-0.2 (#753)
8cf90dd 
* Update the releasing docs (#689)

* Add error reason to output if fail to checkout an account from boskos (#698)

* Temporary workaround a data issue in boskos service (#699)

* Update checkout_account.py to not reuse connections (#700)

* Fix checkout_account.py (#702)

* Make hack/checkin_account.py executable (#703)

* Fix: all traffic ingress rule triggers fatal nil dereference (#697)

* fix: respect all traffic security group rules (and others)

For anything besides tcp, udp, icmp, and icmpv6 there is no applicable
notion of "port range." AWS omits FromPort and ToPort in its responses,
causing a fatal nil dereference when attempting to read any security
groups with e.g. an "all traffic" rule.

* fix: omit description when empty string

* fix: handle more security groups without crashing

This commit cleans up and clarifies a few of the less obvious components
of the previous work.

* fix: handle more security groups without crashing

Address linter failures.

* fix: handle more security groups without crashing

Usage needs to match declaration. Computers are sticklers about that
sort of thing.

* fix: handle more security groups without crashing

Add clarifying comment to serializer function.

* Fixes a bug and adds tests for kubeadm defaults (#707)

The pointers were not working as expected so the API is changing
to be more functional and leverage kubernetes' DeepCopy function.

* Update listed v1.14 AMIs to v1.14.1 (#708)

* Update listed v1.14 AMIs to v1.14.1

* Update README with list of published AMIs/Kubernetes versions

* GZIP user-data (#710)

Signed-off-by: Vince Prignano <vincepri@vmware.com>

* Make sure Calico can talk IP-in-IP (#701)

* MAke sure Calico can talk IP-in-IP

* Add IP in IP protocol to the control plane security group

* Add IPv4 protocol definition and make sure it's handled properly.

* Make port ranges AWS complient and security groups more restrictive.

* Fix security groups

* Adds tests to kubeadm defaults (#709)

Attempt at documenting the assumptions made in the kubeadm
defaults code.

Signed-off-by: Chuck Ha <chuckh@vmware.com>

* Logging (#713)

* Adds logr as dependency

Signed-off-by: Chuck Ha <chuckh@vmware.com>

* Use logr in the cluster actuator

This only creates the logger. Does not yet swap out actual klog calls.

Signed-off-by: Chuck Ha <chuckh@vmware.com>

* update bazel

Signed-off-by: Chuck Ha <chuckh@vmware.com>

* update

Signed-off-by: Chuck Ha <chuckh@vmware.com>

* Switch dep to use release-0.1 branch instead of version (#715)

* Adds logr as dependency (#714)

Adds context for logs and removes excessive logging

Signed-off-by: Chuck Ha <chuckh@vmware.com>

* Ensure `make manifests` generates machines file for HA control plane too. (#720)

* Add HA machines template

* Introduce HA machines file in `make manifests` target

* Add clusterawsadm as make dependency to manifests make target. (#721)

Ensures manifests are generated from the current state of the source.
Assuming $GOPATH/bin is in the $PATH

* Update to Go 1.12 (#719)

Signed-off-by: Vince Prignano <vincepri@vmware.com>

* Add ability to override Organization ID for image lookups (#723)

* Add ability to override Organization ID for image lookups

* Update pkg/cloud/aws/services/ec2/ami.go

Co-Authored-By: detiber <detiberusj@vmware.com>

* Add updated generated crd

* feat: support customizing root device size (#718)

* feat: support customizing root device size

* chore: re-generate CRDs

* fix: update formatting

* chore: add comment describing Service.sdkToInstance

* chore: make service.SDKToInstance public

* Rename BUILD -> BUILD.bazel for consistency (#724)

find . -type file -name BUILD -not -path "./vendor/*" | xargs -n1 -I{} -- git mv {} {}.bazel

Preferred build name changed in 3788fb1
Fixes #722

* Adds retry-on-conflict during updates (#725)

* Adds retry-on-conflict during updates

Signed-off-by: Chuck Ha <chuckh@vmware.com>

* adds note about status update caveat

Signed-off-by: Chuck Ha <chuckh@vmware.com>

* clarify errors/comments

Signed-off-by: Chuck Ha <chuckh@vmware.com>

* Add the HA machines configuration to bazel (#733)

Signed-off-by: Chuck Ha <chuckh@vmware.com>

* Ensure bazel is the correct version (#731)

Signed-off-by: Chuck Ha <chuckh@vmware.com>

* Update OWNERS_ALIASES and SECURITY_CONTACTS (#712)

* Fix the prow jobs (#735)

Signed-off-by: Chuck Ha <chuckh@vmware.com>

* Fix markdown formatting (#736)

* extract fmt from release tool (#738)

Signed-off-by: Chuck Ha <chuckh@vmware.com>

* Use DEFAULT_REGION as the default and REGION as the supplied (#739)

Signed-off-by: Chuck Ha <chuckh@vmware.com>

* e2e testing improvement (#743)

* Bump kind version
* Remove docker load in favor of kind load for e2e cluster

Signed-off-by: Chuck Ha <chuckh@vmware.com>

* fix: Don't try to update root size when it's unset (#726)

* fix: Don't try to update root size when it's unset

This commit looks for empty RootDeviceSize in the spec and ignores it.
Otherwise, none of our control plane machines were updating with this
error:

```
E0418 23:07:48.250925       1 controller.go:214] Error updating machine "ns/controlplane-2": found attempt to change immutable state for machine "controlplane-2": ["Root volume size cannot be mutated from 8 to 0"]
```

* fix: updates without specifying a root volume size

Add unit test.

* fix: updates without specifying a root volume size

Fix gofmt.

* Scope nodeRef to workload cluster (#744)

Signed-off-by: Vince Prignano <vincepri@vmware.com>

* Fix NPE on delete bastion host (#746)

Signed-off-by: Vince Prignano <vincepri@vmware.com>

* Documentation for creating a new cluster on a different AWS account  (#728)

* Initial draft of documentation for Cluster creation using cross account role assumption

* Update roleassumption.md

Complete the document.

* cleanup the documentation for roleassumption

* Resolved the comments: role assumption documentation.

* Fix minor issues - roleassumption.md

* resolve more comments to roleassumption.md

* Resolve more comments - roleassumption.md

* include machines-ha.yaml.template in release artifacts (#741)

* Update AWS sdk, improve log in machine actuator delete (#747)

Signed-off-by: Vince Prignano <vincepri@vmware.com>

* Fixes the infinite reconcile loop (#748)

* Uses patch for updating the cluster and machine specs
  - patch does not cause a re-reconcile in the capi controller
* Uses update for updating the cluster and machine status
  - update for status is ok since it does not update any of the metadata
    no re-reconcile is necessary for the capi controller

Signed-off-by: Chuck Ha <chuckh@vmware.com>

* Update Gopkg.lock and cleanup Makefile (#751)

* Update cluster-api release-0.1 vendor (#750)

Signed-off-by: Vince Prignano <vincepri@vmware.com>

* Reduce the number of re-reconciles (#752)

Signed-off-by: Chuck Ha <chuckh@vmware.com>
@vincepri vincepri deleted the capi-master branch July 26, 2019 17:50
richardchen-db pushed a commit to databricks/cluster-api-provider-aws-1 that referenced this pull request Jan 14, 2023
@detiber @k8s-ci-robot
Backport latest changes to release-0.2 (kubernetes-sigs#753)
be75234 
* Update the releasing docs (kubernetes-sigs#689)

* Add error reason to output if fail to checkout an account from boskos (kubernetes-sigs#698)

* Temporary workaround a data issue in boskos service (kubernetes-sigs#699)

* Update checkout_account.py to not reuse connections (kubernetes-sigs#700)

* Fix checkout_account.py (kubernetes-sigs#702)

* Make hack/checkin_account.py executable (kubernetes-sigs#703)

* Fix: all traffic ingress rule triggers fatal nil dereference (kubernetes-sigs#697)

* fix: respect all traffic security group rules (and others)

For anything besides tcp, udp, icmp, and icmpv6 there is no applicable
notion of "port range." AWS omits FromPort and ToPort in its responses,
causing a fatal nil dereference when attempting to read any security
groups with e.g. an "all traffic" rule.

* fix: omit description when empty string

* fix: handle more security groups without crashing

This commit cleans up and clarifies a few of the less obvious components
of the previous work.

* fix: handle more security groups without crashing

Address linter failures.

* fix: handle more security groups without crashing

Usage needs to match declaration. Computers are sticklers about that
sort of thing.

* fix: handle more security groups without crashing

Add clarifying comment to serializer function.

* Fixes a bug and adds tests for kubeadm defaults (kubernetes-sigs#707)

The pointers were not working as expected so the API is changing
to be more functional and leverage kubernetes' DeepCopy function.

* Update listed v1.14 AMIs to v1.14.1 (kubernetes-sigs#708)

* Update listed v1.14 AMIs to v1.14.1

* Update README with list of published AMIs/Kubernetes versions

* GZIP user-data (kubernetes-sigs#710)

Signed-off-by: Vince Prignano <vincepri@vmware.com>

* Make sure Calico can talk IP-in-IP (kubernetes-sigs#701)

* MAke sure Calico can talk IP-in-IP

* Add IP in IP protocol to the control plane security group

* Add IPv4 protocol definition and make sure it's handled properly.

* Make port ranges AWS complient and security groups more restrictive.

* Fix security groups

* Adds tests to kubeadm defaults (kubernetes-sigs#709)

Attempt at documenting the assumptions made in the kubeadm
defaults code.

Signed-off-by: Chuck Ha <chuckh@vmware.com>

* Logging (kubernetes-sigs#713)

* Adds logr as dependency

Signed-off-by: Chuck Ha <chuckh@vmware.com>

* Use logr in the cluster actuator

This only creates the logger. Does not yet swap out actual klog calls.

Signed-off-by: Chuck Ha <chuckh@vmware.com>

* update bazel

Signed-off-by: Chuck Ha <chuckh@vmware.com>

* update

Signed-off-by: Chuck Ha <chuckh@vmware.com>

* Switch dep to use release-0.1 branch instead of version (kubernetes-sigs#715)

* Adds logr as dependency (kubernetes-sigs#714)

Adds context for logs and removes excessive logging

Signed-off-by: Chuck Ha <chuckh@vmware.com>

* Ensure `make manifests` generates machines file for HA control plane too. (kubernetes-sigs#720)

* Add HA machines template

* Introduce HA machines file in `make manifests` target

* Add clusterawsadm as make dependency to manifests make target. (kubernetes-sigs#721)

Ensures manifests are generated from the current state of the source.
Assuming $GOPATH/bin is in the $PATH

* Update to Go 1.12 (kubernetes-sigs#719)

Signed-off-by: Vince Prignano <vincepri@vmware.com>

* Add ability to override Organization ID for image lookups (kubernetes-sigs#723)

* Add ability to override Organization ID for image lookups

* Update pkg/cloud/aws/services/ec2/ami.go

Co-Authored-By: detiber <detiberusj@vmware.com>

* Add updated generated crd

* feat: support customizing root device size (kubernetes-sigs#718)

* feat: support customizing root device size

* chore: re-generate CRDs

* fix: update formatting

* chore: add comment describing Service.sdkToInstance

* chore: make service.SDKToInstance public

* Rename BUILD -> BUILD.bazel for consistency (kubernetes-sigs#724)

find . -type file -name BUILD -not -path "./vendor/*" | xargs -n1 -I{} -- git mv {} {}.bazel

Preferred build name changed in 3788fb1
Fixes kubernetes-sigs#722

* Adds retry-on-conflict during updates (kubernetes-sigs#725)

* Adds retry-on-conflict during updates

Signed-off-by: Chuck Ha <chuckh@vmware.com>

* adds note about status update caveat

Signed-off-by: Chuck Ha <chuckh@vmware.com>

* clarify errors/comments

Signed-off-by: Chuck Ha <chuckh@vmware.com>

* Add the HA machines configuration to bazel (kubernetes-sigs#733)

Signed-off-by: Chuck Ha <chuckh@vmware.com>

* Ensure bazel is the correct version (kubernetes-sigs#731)

Signed-off-by: Chuck Ha <chuckh@vmware.com>

* Update OWNERS_ALIASES and SECURITY_CONTACTS (kubernetes-sigs#712)

* Fix the prow jobs (kubernetes-sigs#735)

Signed-off-by: Chuck Ha <chuckh@vmware.com>

* Fix markdown formatting (kubernetes-sigs#736)

* extract fmt from release tool (kubernetes-sigs#738)

Signed-off-by: Chuck Ha <chuckh@vmware.com>

* Use DEFAULT_REGION as the default and REGION as the supplied (kubernetes-sigs#739)

Signed-off-by: Chuck Ha <chuckh@vmware.com>

* e2e testing improvement (kubernetes-sigs#743)

* Bump kind version
* Remove docker load in favor of kind load for e2e cluster

Signed-off-by: Chuck Ha <chuckh@vmware.com>

* fix: Don't try to update root size when it's unset (kubernetes-sigs#726)

* fix: Don't try to update root size when it's unset

This commit looks for empty RootDeviceSize in the spec and ignores it.
Otherwise, none of our control plane machines were updating with this
error:

```
E0418 23:07:48.250925       1 controller.go:214] Error updating machine "ns/controlplane-2": found attempt to change immutable state for machine "controlplane-2": ["Root volume size cannot be mutated from 8 to 0"]
```

* fix: updates without specifying a root volume size

Add unit test.

* fix: updates without specifying a root volume size

Fix gofmt.

* Scope nodeRef to workload cluster (kubernetes-sigs#744)

Signed-off-by: Vince Prignano <vincepri@vmware.com>

* Fix NPE on delete bastion host (kubernetes-sigs#746)

Signed-off-by: Vince Prignano <vincepri@vmware.com>

* Documentation for creating a new cluster on a different AWS account  (kubernetes-sigs#728)

* Initial draft of documentation for Cluster creation using cross account role assumption

* Update roleassumption.md

Complete the document.

* cleanup the documentation for roleassumption

* Resolved the comments: role assumption documentation.

* Fix minor issues - roleassumption.md

* resolve more comments to roleassumption.md

* Resolve more comments - roleassumption.md

* include machines-ha.yaml.template in release artifacts (kubernetes-sigs#741)

* Update AWS sdk, improve log in machine actuator delete (kubernetes-sigs#747)

Signed-off-by: Vince Prignano <vincepri@vmware.com>

* Fixes the infinite reconcile loop (kubernetes-sigs#748)

* Uses patch for updating the cluster and machine specs
  - patch does not cause a re-reconcile in the capi controller
* Uses update for updating the cluster and machine status
  - update for status is ok since it does not update any of the metadata
    no re-reconcile is necessary for the capi controller

Signed-off-by: Chuck Ha <chuckh@vmware.com>

* Update Gopkg.lock and cleanup Makefile (kubernetes-sigs#751)

* Update cluster-api release-0.1 vendor (kubernetes-sigs#750)

Signed-off-by: Vince Prignano <vincepri@vmware.com>

* Reduce the number of re-reconciles (kubernetes-sigs#752)

Signed-off-by: Chuck Ha <chuckh@vmware.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@justinsb justinsb Awaiting requested review from justinsb

2 more reviewers

@chuckha chuckha chuckha left review comments

@detiber detiber detiber left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@detiber detiber

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@vincepri @k8s-ci-robot @detiber @chuckha