Skip to content

Conversation

@wking
Copy link
Member

@wking wking commented Jul 9, 2018

Both of these lines were touched by ae41b0a (#22), but there's no need to use Sprintf to inject string literals into the format template. 26930f2 (#22) fixed one such instance, but left these. The * instance dates back to bf61dc9 (#16).

@coreosbot
Copy link

Can one of the admins verify this patch?

@openshift-ci-robot openshift-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jul 9, 2018
@smarterclayton
Copy link
Contributor

/ok-to-test

@openshift-ci-robot openshift-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jul 10, 2018
Both of these lines were touched by ae41b0a (Merge pull request openshift#22
from trawler/kube_tls_certs, 2018-07-09), but there's no need to use
Sprintf to inject string literals into the format template.  26930f2
(post-review fixes, 2018-07-07, openshift#22) fixed one such instance, but left
these.  The * instance dates back to bf61dc9
(installer/pkg/config-generator/tls: generate ingress certs,
2018-07-06, openshift#16).
@wking wking force-pushed the sprintf-string-literals branch from 0e14846 to 3650e3d Compare July 10, 2018 02:40
@wking
Copy link
Member Author

wking commented Jul 10, 2018

Similar 31 errors in the e2e-aws tests as we saw here:

Failure [104.628 seconds]
[BeforeSuite] BeforeSuite 
/tmp/openshift/build-rpms/rpm/BUILD/origin-3.11.0/_output/local/go/src/github.com/openshift/origin/vendor/k8s.io/kubernetes/test/e2e/e2e.go:140

  Expected error:
      <*url.Error | 0xc421174b40>: {
          Op: "Get",
          URL: "https://ci-op-vg01kfty-68485-api.origin-ci-int-aws.dev.rhcloud.com:6443/api/v1/namespaces/kube-system/events",
          Err: {s: "EOF"},
      }
      Get https://ci-op-vg01kfty-68485-api.origin-ci-int-aws.dev.rhcloud.com:6443/api/v1/namespaces/kube-system/events: EOF
  not to have occurred

  /tmp/openshift/build-rpms/rpm/BUILD/origin-3.11.0/_output/local/go/src/github.com/openshift/origin/vendor/k8s.io/kubernetes/test/e2e/framework/util.go:2399

The difference is EOF (this time) vs. TLS handshake timeout (in #46).

@abhinavdahiya
Copy link
Contributor

/test e2e-aws

@wking
Copy link
Member Author

wking commented Jul 11, 2018

The two e2e-aws failures look like these.

Copy link
Contributor

@yifan-gu yifan-gu left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 11, 2018
@yifan-gu
Copy link
Contributor

/retest

@openshift-merge-robot openshift-merge-robot merged commit b669e57 into openshift:master Jul 20, 2018
@wking wking deleted the sprintf-string-literals branch July 20, 2018 22:42
stbenjam pushed a commit to stbenjam/installer that referenced this pull request Feb 10, 2021
Update owners file with addition of asalkeld
mkumatag pushed a commit to mkumatag/installer that referenced this pull request Aug 17, 2021
Deprecation warnings are currently thrown because of quoted
references to the ibm.vpc and ibm.powervs providers. The
reccomendation since terraform 0.12.14 is to remove any
quotations. [0]

[0] https://discuss.hashicorp.com/t/terraform-0-12-14-released/389
clnperez pushed a commit to clnperez/installer that referenced this pull request Sep 2, 2021
Deprecation warnings are currently thrown because of quoted
references to the ibm.vpc and ibm.powervs providers. The
reccomendation since terraform 0.12.14 is to remove any
quotations. [0]

[0] https://discuss.hashicorp.com/t/terraform-0-12-14-released/389
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm Indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants