Watch for the service account token to be ready

[nitishm]

Instead of using a sleep use an actual watcher to ensure that the token
is ready before proceeding.

Closes #403

Signed-off-by: Nitish Malhotra nitish.malhotra@gmail.com

[bacongobbler]

Hi @nitishm. Sorry for the late response.

Generally speaking we ask for unit tests to accompany any changes. That way we can ensure future changes won't introduce a code regression, and we have some context around why certain changes were made in the past. Would you mind writing some tests? Thanks!

[bacongobbler]

Looking at this closer... It might be too difficult to implement tests for the tests. I think we can make an exception here.

[thomastaylor312]

Looking great, just one minor change to remove old commented out code and the associated TODO

[thomastaylor312]

Looks like tests are failing due to cargo fmt. You should just be able to run cargo fmt --all and things should be good after that

[thomastaylor312]

Looks like you have a compilation error (and some warnings) in the new tests you added:

warning: unused import: `Metadata`
 --> tests/test_resource_manager.rs:6:5
  |
6 |     Metadata,
  |     ^^^^^^^^
  |
  = note: `#[warn(unused_imports)]` on by default

warning: unused import: `WatchEvent`
 --> tests/test_resource_manager.rs:9:54
  |
9 |     api::{Api, DeleteParams, ListParams, PostParams, WatchEvent},
  |                                                      ^^^^^^^^^^

error[E0599]: no method named `first` found for enum `std::option::Option` in the current scope
   --> tests/test_resource_manager.rs:125:40
    |
125 |                 let secret = o.secrets.first().unwrap().clone();
    |                                        ^^^^^ method not found in `std::option::Option<Vec<ObjectReference>>`

warning: unused import: `ResourceExt`
  --> tests/test_resource_manager.rs:10:5
   |
10 |     ResourceExt,
   |     ^^^^^^^^^^^

[thomastaylor312]

Hey there @nitishm! Sorry for the delay. We were having some e2e test problems that should probably solve your issue. Mind rebasing now that #694 is merged? I can do it for you as well if you don't mind me force pushing

[nitishm]

Go for it! I don't mind you force pushing

[thomastaylor312]

Looks like this is causing e2e test failures. Maybe this should retry for up to 10s before erroring