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

Best way to exit a watch on success #15

Open
cknowles opened this issue Aug 11, 2016 · 11 comments
Open

Best way to exit a watch on success #15

cknowles opened this issue Aug 11, 2016 · 11 comments

Comments

@cknowles
Copy link
Contributor

When watching for a success case, such as a deployment finishing, what is the recommended way to signal the close of a watch? I tried a few possible options but nothing exited the watch as expected so far.

var res = kubeapi.watch('watch/namespaces/default/pods', function(data) {
    // some condition on data means we want to exit now
    // how do we signal here so we can call a callback with success?
    // e.g. callback(null, data);
}, function(err) {
    // error during deploy
    callback(err);
});

setTimeout(function() {
    // timeout if not successful after some time
    res.emit('close');
}, 30000);
@mmoonn2
Copy link
Member

mmoonn2 commented Aug 11, 2016

I do not understand. you want to this?

var res = kubeapi.watch('watch/namespaces/default/pods', function(data){
    if( true ){
          res.emit('close')
          callback(null. data)
    }
}, function(err){
    // if use res.emit('close') .  err is undefined
    if( err ) // timeout or other error
         callback(err)
}, 30000)
// last param is timeout

@cknowles
Copy link
Contributor Author

I tried something very similar by emitting a close signal inside the success block but it didn't seem to exit correctly, I will double check it tomorrow and send the exact code I'm using.

@cknowles
Copy link
Contributor Author

I tried again today, the trouble seems to be the example code still waits until the timeout time even with success. Emitting the close signal in the data block does not seem to actually close it.

What I mean is even if success is after 1 second, the block waits for the entire timeout until the process finishes. e.g. 30 seconds in our case above.

Not much difference in the code I'm trying:

var res = kubeapi.watch(`watch/namespaces/${namespace}/${resourceType}/${name}`,
  function(data) {
    if (isDeployFinished(data)) {
      res.emit('close');
      callback(null, data);
    } else {
      console.log('Deployment not finished yet');
    }
  },
  function(err) {
    if (err) {
      console.log(`Error while waiting for deployment of ${name}`, err);
    }
    callback(err);
  }, 30000);

@mmoonn2
Copy link
Member

mmoonn2 commented Aug 12, 2016

which the version of k8s module?

@cknowles
Copy link
Contributor Author

0.4.5

@mmoonn2
Copy link
Member

mmoonn2 commented Aug 12, 2016

you code will run success callback twice. because res.emit('close') will execute error callback.

  function(err) {
    if (err) {
      // timeout or other error
      console.log(`Error while waiting for deployment of ${name}`, err);
      callback(err); // this error callback
    }else{
         // call res.emit('close')  will enter this block.  you should not call callback
    }
  }, 30000);

@cknowles
Copy link
Contributor Author

OK I see. However, I just tried that but it still waits for the full 30 seconds even though the deployment finishes after 1-2 seconds (I added some extra logging for that to, I'm hitting the res.emit line ok). My callback code executes fine and exits so it seems the timeout is hanging around without being cancelled on the close signal.

@mmoonn2
Copy link
Member

mmoonn2 commented Aug 12, 2016

this is my test code . It works. error callback call only once

var res = kubeapi.watch('watch/namespaces/default/pods', function(data){

}, function(err){
    console.log(err)
}, 20000)

setTimeout(function() {
    console.log('close')
    res.emit('close')
}, 10000);

@cknowles
Copy link
Contributor Author

But how do you exit immediately on success in the function(data)? Your above example is empty in that block so it's possible to timeout but I'm looking for immediate exit on success case.

@cknowles
Copy link
Contributor Author

cknowles commented Sep 2, 2016

Came back to this to try to resolve it. I wanted to clarify the problem we have to see if I missed something. I tried to simplify the code to remove the callbacks so I can show the issue we encountered:

const namespace = 'foo';
const resourceType = 'deployments';
const name = 'bar';

console.log('%s Starting', Date.now());
var res = kubeapi.watch(`watch/namespaces/${namespace}/${resourceType}/${name}`,
  function(data) {
    console.log('%s Data update for wait %j', Date.now(), data);
    res.emit('close');
  },
  function(err) {
    if (err) {
      console.log('%s Error while waiting %s', Date.now(), err);
    } else {
      console.log('%s Finished without error', Date.now());
    }
  }, 30000);

Running on the terminal:

✗ node --version
v6.3.1
✗ time node src/test.js

Prints this:

1472812165454 Starting
1472812166596 Data update for wait [...]
1472812166596 Finished without error
node src/test.js  0.51s user 0.11s system 1% cpu 31.924 total

Maybe I've missed some mechanism to exit properly. As you can see, the code seems to be finished after ~1 second since the exit function runs but the process itself remains and only exits after another 30 seconds (the timeout time).

@mmoonn2
Copy link
Member

mmoonn2 commented Sep 5, 2016

If you do not use unittest run it, I believe it will exit immediately

test.js

const K8s = require('k8s')

const kubeapi = K8s.api({
    endpoint: 'https://192.168.99.101:8443',
    version: '/api/v1',
    auth: {
        clientCert: fs.readFileSync(`${process.env.HOME}/.minikube/apiserver.crt`).toString(),
        clientKey: fs.readFileSync(`${process.env.HOME}/.minikube/apiserver.key`).toString() ,
        caCert: fs.readFileSync(`${process.env.HOME}/.minikube/ca.crt`).toString()
    }
})

const namespace = 'foo';
const resourceType = 'deployments';
const name = 'bar';

console.log('%s Starting', Date.now());
var res = kubeapi.watch(`watch/namespaces/${namespace}/${resourceType}/${name}`,
  function(data) {
    console.log('%s Data update for wait %j', Date.now(), data);
    res.emit('close');
    res.emit('close');
  },
  function(err) {
    if (err) {
      console.log('%s Error while waiting %s', Date.now(), err);
    } else {
      console.log('%s Finished without error', Date.now());
    }
  }, 15000);

node -v 
//v5.10.1
node test.ts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants