vmware / vic

vSphere Integrated Containers Engine is a container runtime for vSphere.
http://vmware.github.io/vic
Other
640 stars 173 forks source link

Enforce doc impact assessment before closing issues #4991

Open stuclem opened 7 years ago

stuclem commented 7 years ago

Per https://github.com/vmware/vic-product/issues/195, let's do a trial whereby assessing issues for doc impact is a part of the formal workflow for closing issues. We can start with /vic and then extend the process to /admiral and /harbor if it works.

To set this up, we need the following:

mdubya66 commented 7 years ago

The enforcing part needs some thought. Developers don't really close issues, they merge PR's which closes the issue.

mdubya66 commented 7 years ago

The team will discuss this during the retrospective

mhagen-vmware commented 7 years ago

Making this high per our discussion in retro. We have agreed to implement it as proposed.

gigawhitlocks commented 7 years ago

I wrote a utility that solves this problem, in the PR linked above. Still needs addition to the existing nginx/ansible configuration so that certs are handled properly, a GitHub account needs to be created for the utility to act through, the VIC repo needs to have the webhook added after the utility is deployed, and the utility needs to actually be deployed & the process for doing so documented.

There are a number of ways to deploy this utility, and there are significant drawbacks to all of them. Adding config to Ansible and shoehorning this service into one of the existing nginx containers is one way, and that alleviates the need for this service to manage its own certs and also allows us to store the two tokens necessary in the Ansible vault, however this would be overloading the usage of an existing nginx config and this approach will require editing the Dockerfile used by Ansible for CI, which lives in another repository altogether.

Alternatively, the service could manage its own certificates, which the current version does via autocert, however autocert only opens on port 443 (hardcoded) so likewise this needs to be wrapped in a container so that it can run on a nonstandard port, which will need to be opened in the firewall. The binary would be distributed in this case as a docker image. This approach also requires manual management of secrets, since it's an option that presumably would happen outside of Ansible, although that would break the idiomatic approach used elsewhere in the CI repository.

Or, we could run it in a container as above and use the docker-compose file used to spin up a different nginx instance on a different VM. This approach also requires that we handle secrets manually.

It may be simplest to simply spin up a new VM in the datacenter to handle this process, but that's massive overkill for a single go binary that handles one web request.

In the interest of time I'm putting this back in To Do so more pressing issues can be addressed in the meantime, and perhaps someone more familiar with the CI repository can comment on the best path forward or take lead on the issue themselves.

Building the binary, should someone want to pick this issue up where I left it, is as simple as go get-ing the 3 dependencies and calling go build and instructions for running the utility are present in a comment in main.go.

gigawhitlocks commented 7 years ago

All-in-all this whole task is far more complicated than it really has any right to be, mostly because of what I would describe as technical debt. Sad panda.

casualjim commented 7 years ago

this is deployed at: https://doclabels.vmware.run/whoami

The source and dockerfile used for this are here: https://github.com/casualjim/enforce-doc-labels I removed the autocert because it is too greedy for the 443 port and run ssl termination on the nginx proxy in front of this.

the github related configuration still needs to happen and then we have to restart that container with the right environment vars

casualjim commented 7 years ago

I've configured the webhook on the github repository

mdubya66 commented 7 years ago

needs some final touches before calling this done