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.

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

Update to Go 1.12 #719

Merged
merged 1 commit into from
Apr 16, 2019
Merged

Conversation

vincepri
Copy link
Member

Signed-off-by: Vince Prignano [email protected]

What this PR does / why we need it:
This PR updates build tools to Go 1.12

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 #623

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/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 15, 2019
@k8s-ci-robot
Copy link
Contributor

[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

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 Apr 15, 2019
@vincepri vincepri force-pushed the update-go-112 branch 2 times, most recently from bf3e5d1 to 2d0e609 Compare April 16, 2019 00:44
@k8s-ci-robot k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 16, 2019
@vincepri
Copy link
Member Author

/assign @randomvariable @detiber

Copy link
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.

thanks! next stop, go mod~

/lgtm

@@ -312,7 +312,7 @@ func (s *Service) describeClassicELB(name string) (*v1alpha1.ClassicELB, error)
}
}

if out == nil && len(out.LoadBalancerDescriptions) == 0 {
if out != nil && len(out.LoadBalancerDescriptions) == 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

nice

Copy link
Member Author

Choose a reason for hiding this comment

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

go vet 1.12 is smarter than ever

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 16, 2019
@detiber
Copy link
Member

detiber commented Apr 16, 2019

Why the rename here: pkg/cloud/aws/services/ec2/BUILD → pkg/cloud/aws/services/ec2/BUILD.bazel

@vincepri
Copy link
Member Author

/retest

@vincepri
Copy link
Member Author

@detiber No reason, I think I deleted and it regenerated the file with the new naming convention Bazel has

@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 16, 2019
@detiber
Copy link
Member

detiber commented Apr 16, 2019

I thought we standardized on BUILD with no suffix because of an issue we were hitting with osx and case sensitivity. @chuckha or @randomvariable do either of you remember?

@vincepri
Copy link
Member Author

Whispers words of encouragement to bazel and mocks

@chuckha
Copy link
Contributor

chuckha commented Apr 16, 2019

@detiber It was an edge case we encountered in a vendor directory. The project had a build/ directory and the BUILD file caused chaos on OS X because case insensitivity as you suggested.

Either is fine so long as there is not a conflicting directory.

@vincepri
Copy link
Member Author

I thought it was the vice-versa, we're preferring BUILD.bazel to just BUILD for the above reason, wasn't it?

@chuckha
Copy link
Contributor

chuckha commented Apr 16, 2019

edit; iirc we actually preferred BUILD.bazel for this reason, but it usually doesn't matter

@justaugustus
Copy link
Member

@vincepri
Copy link
Member Author

@vincepri
Copy link
Member Author

/retest

@vincepri
Copy link
Member Author

/test pull-cluster-api-provider-aws-bazel-integration

Signed-off-by: Vince Prignano <[email protected]>
@randomvariable
Copy link
Member

It was BUILD.bazel to deal with the case insensitivity issue.

We really should rename all the BUILD files to BUILD.bazel for consistency.

Needn't be this PR though (see #722).

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 16, 2019
@k8s-ci-robot k8s-ci-robot merged commit 1ba422e into kubernetes-sigs:master Apr 16, 2019
detiber pushed a commit to detiber/cluster-api-provider-aws that referenced this pull request May 2, 2019
k8s-ci-robot pushed a commit that referenced this pull request May 2, 2019
* 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 <[email protected]>

* 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 <[email protected]>

* Logging (#713)

* Adds logr as dependency

Signed-off-by: Chuck Ha <[email protected]>

* Use logr in the cluster actuator

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

Signed-off-by: Chuck Ha <[email protected]>

* update bazel

Signed-off-by: Chuck Ha <[email protected]>

* update

Signed-off-by: Chuck Ha <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* adds note about status update caveat

Signed-off-by: Chuck Ha <[email protected]>

* clarify errors/comments

Signed-off-by: Chuck Ha <[email protected]>

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

Signed-off-by: Chuck Ha <[email protected]>

* Ensure bazel is the correct version (#731)

Signed-off-by: Chuck Ha <[email protected]>

* Update OWNERS_ALIASES and SECURITY_CONTACTS (#712)

* Fix the prow jobs (#735)

Signed-off-by: Chuck Ha <[email protected]>

* Fix markdown formatting (#736)

* extract fmt from release tool (#738)

Signed-off-by: Chuck Ha <[email protected]>

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

Signed-off-by: Chuck Ha <[email protected]>

* e2e testing improvement (#743)

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

Signed-off-by: Chuck Ha <[email protected]>

* 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 <[email protected]>

* Fix NPE on delete bastion host (#746)

Signed-off-by: Vince Prignano <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

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

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

Signed-off-by: Vince Prignano <[email protected]>

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

Signed-off-by: Chuck Ha <[email protected]>
@vincepri vincepri deleted the update-go-112 branch July 26, 2019 17:57
richardchen-db pushed a commit to databricks/cluster-api-provider-aws-1 that referenced this pull request Jan 14, 2023
* 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 <[email protected]>

* 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 <[email protected]>

* Logging (kubernetes-sigs#713)

* Adds logr as dependency

Signed-off-by: Chuck Ha <[email protected]>

* Use logr in the cluster actuator

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

Signed-off-by: Chuck Ha <[email protected]>

* update bazel

Signed-off-by: Chuck Ha <[email protected]>

* update

Signed-off-by: Chuck Ha <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* adds note about status update caveat

Signed-off-by: Chuck Ha <[email protected]>

* clarify errors/comments

Signed-off-by: Chuck Ha <[email protected]>

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

Signed-off-by: Chuck Ha <[email protected]>

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

Signed-off-by: Chuck Ha <[email protected]>

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

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

Signed-off-by: Chuck Ha <[email protected]>

* Fix markdown formatting (kubernetes-sigs#736)

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

Signed-off-by: Chuck Ha <[email protected]>

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

Signed-off-by: Chuck Ha <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

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

Signed-off-by: Vince Prignano <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

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

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

Signed-off-by: Vince Prignano <[email protected]>

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

Signed-off-by: Chuck Ha <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update to Go 1.12
6 participants