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.

Sign up for GitHub

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

Jump to bottom

[docker-ptf]: Update entrypoint entry for docker-ptf #836

Merged

pavel-shirshov merged 20 commits into sonic-net:master from pavel-shirshov:pavelsh/fix_broken_ptf

Jul 24, 2017

dockers/docker-ptf/Dockerfile.j2

Original file line number	Diff line number	Diff line change
Expand Up		@@ -100,4 +100,4 @@ COPY ["supervisord.conf", "sshd.conf", "ptf_nn_agent.conf", "/etc/supervisor/con

Copy link Collaborator lguohan Jul 24, 2017 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. it looks for supervisord.conf in /etc/supervisor/ Copy link Contributor jleveque Jul 24, 2017 • edited Loading Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. There is a "main" config file in /etc/supervisor/, and that file includes the /etc/supervisor/conf.d/ path, which is where the docker-specific config file lives. I will say that naming the docker-specific config file "supervisor.conf" does make things a bit confusing. We could rename the docker-specific config files at some point to make it a bit more obvious. Copy link Contributor Author pavel-shirshov Jul 24, 2017 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. I can rename conf.d/supervisord.conf file to something else. But I think it's good enough name to show that we're changing supervisord configuration in the file. Copy link Contributor jleveque Jul 24, 2017 • edited Loading Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. Currently, the conf.d/ file is only partial configuration, with the remaining (shared) configuration in /etc/supervisord.conf. Thus, I feel like the partial config file in conf.d should have a name specific to what it's configuring (along the lines of "container.conf"). This is a common convention for all /etc/*/conf.d/ config files. The fact that it resides under /etc/supervisor/conf.d/ should be enough to signify that it is modifying the supervisord configuration. See my comment below for more.
		EXPOSE 22

		ENTRYPOINT ["/usr/bin/supervisord"]
		ENTRYPOINT ["/usr/local/bin/supervisord"]