Fix VM DNS in tests #42450
Fix VM DNS in tests #42450
Conversation
howardjohn
added
release-notes-none
istio-testing
added
the
size/XS
|
/test integ-telemetry |
istio-testing
added
size/M
| return &gcpEnv{} | ||
| return &gcpEnv{ | ||
| fillMetadata: lazy.New(func() (bool, error) { | ||
| return shouldFillMetadata(), nil |
There was a problem hiding this comment.
Should this be e.shouldFIllMetadata?
Or actually, isn't this a circular definition? It looks like gcpEnv.fillMetadata depends on shouldFillMetadata(), but that function depends on the existence of e.fillMetadata.
I may be missing something.
There was a problem hiding this comment.
shouldFillMetadata() can probably be inlined and then its less confusing
There was a problem hiding this comment.
Ah its because we mock it, forgot about that
| const metadataIP = "169.254.169.254" | ||
| host := os.Getenv(metadataHostEnv) | ||
| if host == "" { | ||
| host = metadataIP |
There was a problem hiding this comment.
As mentioned in chat, I think we need to check the additional condition that the header of the returned result matches the GCP metadata flavor header.
There was a problem hiding this comment.
I think if we do that we need to completely replace OnGCE. I might not have time to cover that in the short term future, does it make sense as a followup? Since right now tests are broken. The current change should be strictly an improvement
|
In response to a cherrypick label: #42450 failed to apply on top of branch "release-1.14": |
|
In response to a cherrypick label: new issue created for failed cherrypick: #42644 |
|
In response to a cherrypick label: new pull request created: #42645 |
|
In response to a cherrypick label: new pull request created: #42646 |
* Fix VM DNS in tests Fixes istio#42449 * fix isMetadataEndpointAccessible as well * fix port (cherry picked from commit 9c6ba28)
* Fix VM DNS in tests Fixes istio#42449 * fix isMetadataEndpointAccessible as well * fix port
Fixes #42449
Fixes #42452
Note because of #42452 this test has been broken of GCP forever. It only recently broke on GCP due to #42449
Please provide a description of this PR: