fix the inconsistency handling between infra image and normal task image

[fredwangwang]

fix a minor inconsistency in how nomad handles image url for task and infra.

basically a duplication of

nomad/drivers/docker/driver.go

Lines 253 to 256 in 2176fbd

	// Remove any http
	if strings.HasPrefix(driverConfig.Image, "https://") {
	driverConfig.Image = strings.Replace(driverConfig.Image, "https://", "", 1)
	}

[hashicorp-cla]

All committers have signed the CLA.

[tgross]

We perform this operation on the main image just before starting the task. It feels like it'd make sense to either:

• Do this in driver/docker/network.go where we pull the infra image.
• Do both the main image and infra image in this spot.

[fredwangwang]

nomad/drivers/docker/network.go

Line 22 in a40a140

repo, _ := parseDockerImage(d.config.InfraImage)

I actually tried to put it in network.go at beginning haha. But quite some places read *d.config.InfraImage directly, so if I want those methods to take the change, I need to mutate global struct state. If I mutate the global state, if doesn't feel right to do it every time before pull infra. So I put it here.

@tgross could you help to put it where you think fits most? It's a minor inconsistency, which was causing some confusion to us, but def not a blocker.

[tgross]

If I mutate the global state, if doesn't feel right to do it every time before pull infra.

Yeah, I'm inclined to agree with you there. I know we had a couple PRs recently around the infrastructure images, so I want to make sure I get the opinion of folks who worked on that. @shoenig what do you think about moving the http:// stripping for the infra images here too?

[shoenig]

Seems fine to me. I do feel that both code blocks should be using https://golang.org/pkg/strings/#TrimPrefix

[tgross]

Good call. @fredwangwang sorry about the delay in getting back on this but do you think you can make that change? If not, happy to carry this PR.

[fredwangwang]

go for it @tgross, and thanks for getting back to this issue!

[tgross]

It turns out that those two sections of code didn't both have access to the same variables, so it doesn't make sense to handle them in the same place after all. I've made @shoenig's suggestion to use TrimPrefix, rebased this PR on master, and added a changelog entry.

I've carried the PR

[tgross]

Bah, I meant to squash that. But this is merged and will ship in the next normal patch release.

[github-actions]

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.