Correctly handle reading from events channel

[crosbymichael]

Fixes #2308

Channel handling was backwards, the bool represents more not closed.

Signed-off-by: Michael Crosby crosbymichael@gmail.com

[crosbymichael]

FYI. This is a ctr only bug in 1.1. I don't think anything else is affected.

[dmcgowan]

This is guaranteed to never return nil for e?

[crosbymichael]

you need a separate check for e != nil. the bool just tells you if the channel is still open or not, it does not signal what the value returned is

[crosbymichael]

added this check

[codecov-io]

Codecov Report

Merging #2309 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #2309   +/-   ##
=======================================
  Coverage   45.54%   45.54%           
=======================================
  Files          83       83           
  Lines        9185     9185           
=======================================
  Hits         4183     4183           
  Misses       4326     4326           
  Partials      676      676

Flag	Coverage Δ
#linux	50.07% <ø> (ø)	⬆️
#windows	41.24% <ø> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 209a7fc...0906879. Read the comment docs.

[aanm]

@crosbymichael I reported the bug because it was the easiest way to replicate it.

I tried running a small code snippet with your changes but I get this

err: type with url containerd.events.ContainerCreate: not found

func main() {
     	c, err := containerd.New(endpoint)
	if err != nil {
		return err
	}
	for {
		eventsCh, errCh := c.EventService().Subscribe(ctx.Background(), `topic~="/containers/create"`)
		err := listenEvents(eventsCh, errCh)
		fmt.Printf("err: %s\n", err)
	}
}

func listenEvents(eventsCh <-chan *events.Envelope, errCh <-chan error) error {
	open := true
	for open {
		var e *events.Envelope
		select {
		case e, open = <-eventsCh:
		case err := <-errCh:
			return err
		}

		var out []byte
		if e.Event != nil {
			v, err := typeurl.UnmarshalAny(e.Event)
			if err != nil {
				return err
			}
			out, err = json.Marshal(v)
			if err != nil {
				return err
			}
		}

		if _, err := fmt.Println(
			e.Timestamp,
			e.Namespace,
			e.Topic,
			string(out),
		); err != nil {
			return err
		}
	}
	return nil
}

[crosbymichael]

@aanm i tried your snipped and it works for me. That error you are getting now is unrelated to this change and maybe a mismatch of versions on your system when you are compiling a 1.1 containerd and and older client, or vise versa, it's hard to tell.

[dmcgowan]

@crosbymichael I noticed I fixed this is a slightly different way when updating the code for moby https://github.com/moby/moby/pull/36895/files

The Subscribe function never actually closes the event channel, just the err channel. In that case I am not just ignoring a nil event and going back to the select, waiting for the error channel to signal completion.

[crosbymichael]

@dmcgowan ok, i'll update it tomorrow morning

[Random-Liu]

LGTM. This is my mistake...

[crosbymichael]

@dmcgowan updated to handle close from the error chan.

[estesp]

LGTM

[dmcgowan]

LGTM