Documentation - Update Go dependency management doc

[s-fairchild]

Which issue this PR addresses:

N/A

Fixes

What this PR does / why we need it:

Update documentation to remove instructions to run make vendor. This script was previously required when the ARO installer was vendored, as well as other OCP 4.x dependencies.

Test plan for issue:

Updated Go module dependencies recently with these steps.

Is there any documentation that needs to be updated for this PR?

N/A

[cadenmarchese]

Should the makefile target be deprecated / removed altogether? I'm curious on if we still need it for anything if we're removing it from docs.

[s-fairchild]

I was asking myself the same question. I see Ben updated the script a few weeks ago PR #3301

He said it still has some uses, so I don't think we should remove it. Documentation on it's specific purposes and uses would be nice.

[kimorris27]

Don't certain OCP repos still need to be vendored using the script? I'm looking at

ARO-RP/go.mod

Line 740 in 21f0e23

// Installer dependencies. Some of them are being used directly in the RP.

on master and that's why I'm thinking the script is still needed.

But if the premise that we no longer need the script holds, the doc updates LGTM.

[s-fairchild]

Don't certain OCP repos still need to be vendored using the script? I'm looking at

ARO-RP/go.mod

Line 740 in 21f0e23

// Installer dependencies. Some of them are being used directly in the RP.

on master and that's why I'm thinking the script is still needed.

But if the premise that we no longer need the script holds, the doc updates LGTM.

We flushed out that this documentation is outdated in this thread.

Maybe I should remove that comment from go.mod too?

[kimorris27]

Don't certain OCP repos still need to be vendored using the script? I'm looking at

ARO-RP/go.mod

Line 740 in 21f0e23

// Installer dependencies. Some of them are being used directly in the RP.

on master and that's why I'm thinking the script is still needed.
But if the premise that we no longer need the script holds, the doc updates LGTM.

We flushed out that this documentation is outdated in this thread.

Maybe I should remove that comment from go.mod too?

Gotcha, I read through the thread. Based on #3301, it looks like @bennerv didn't remove the whole script, just the part that previously vendored in the custom installer. And reading through the comments at the top of the script (see below), it's not clear to me why we're considering the script deprecated, since the logic explained there seems that it would still apply despite the recent installer-related changes.

So I'm not even sure about the go.mod comment tbh. My intention in going back and forth here is just to help guide us to a point where we're fully confident we're not prematurely deprecating something that we still need.

ARO-RP/hack/update-go-module-dependencies.sh

Lines 5 to 21 in 21f0e23

	# TLDR: OCP consists of many repos where for each release a new release branch gets created (release-X.Y).
	# When we update vendors we want to get latest changes from the release branch for all of the dependencies.
	# With Go modules we can't easily do it, but there is a workaround which consists of multiple steps:
	# 1. Get the latest commit from the branch using `go list -mod=mod -m [email protected]`.
	# 2. Using `sed`, transform output of the above command into format accepted by `go mod edit -replace`.
	# 3. Modify `go.mod` by calling `go mod edit -replace`.
	#
	# This needs to happen for each module that uses this branching strategy: all these repos
	# live under github.com/openshift organisation.
	#
	# There are however, some exceptions:
	# * Some repos under github.com/openshift do not use this strategy.
	# We should skip them in this script and manage directly with `go mod`.
	# * Some dependencies pin their own dependencies to older commits.
	# For example, dependency Foo from release-4.7 branch requires
	# dependency Bar at older commit which is
	# not compatible with [email protected].

[SudoBrendan]

I think this introduces some changes that will impact SREs on various OS. Can we get a +1 from both sides?

[SudoBrendan]

My work laptop is fubar so I can't test this myself, but I am 99% sure this breaks re: mac vs linux. Can we get an SRE from each side of the aisle to attempt this change?

[SudoBrendan]

Woops, ok, so maybe it was a Linux thing - have my laptop back up and I, for reasons unknown along my dev journey on this team, have changed my GO111MODULE="", I can test this now, so I'll give it a go and see what happens :)

[SudoBrendan]

Same as above.

[s-fairchild]

@bennerv Can you review this PR when you have a chance?
There's a question in this comment I think you maybe able to answer.