*: move to Ignition v3

[runcom]

• we're still waiting on a tag to fix Gopkg.toml to use a tag (News 2.0.0 coreos/ignition#773)
• I still have to polish functions and address some todos but want to see if logic is right first

Signed-off-by: Antonio Murdaca [email protected]

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: runcom
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: crawford

If they are not already assigned, you can assign the PR to them by writing /assign @crawford in a comment when ready.

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[runcom]

/hold

This is not going to pass until MCO/RHCOS are done with the move as well

@wking @abhinavdahiya @crawford @ashcrow @cgwalters @jlebon @ajeddeloh ptal

[wking]

Can you split the vendor bump out into a separate commit (like this), so we can more easily review the changes you made without having the changes dep made mixed in?

[runcom]

Can you split the vendor bump out into a separate commit (like this), so we can more easily review the changes you made without having the changes dep made mixed in?

done

[wking]

Instead of setting up ign and then, in the append case, immediately clobbering it, I'd rather have:

var ign whateverTypeThisShouldBe
if appendToFile {
  ign = ignition.FileAppendFromBytes(strings.TrimSuffix(base, ".template"), "root", mode, data)
} else {
  ign = ignition.FileFromBytes(strings.TrimSuffix(base, ".template"), "root", mode, data)
}

Or we should update FileFromBytes to support an append parameter, or an options struct parameter, or some such...

[runcom]

yeah, I've already accounted for that, just want to validate that the v3 way of doing append is how I did @ajeddeloh

[ajeddeloh]

yeah this looks good.

[ajeddeloh]

Actually, in the case of appending you'll want to make sure a file entry doesn't already exist at that path. In that case you'll need to add the append entry to it's append list.

[wking]

Actually, in the case of appending you'll want to make sure a file entry doesn't already exist at that path.

This helper is for our internal use, so we can probably ignore that case. Will Ignition crash and burn if this happens, so we can fail out in CI if by some programmer mistake we have two entries for the same file?

[ajeddeloh]

Ignition will crash and burn at validation time (note: you can call the validation function ahead of time) since there will be two file entries with the same path

[abhinavdahiya]

nit: no need for func.
https://play.golang.org/p/kM-DWZ9yCXN

package main

import (
	"fmt"
	"net/url"
)

func main() {
	u := (&url.URL{
		Scheme: "https",
		Host:   fmt.Sprintf("api.%s:22623", "adahiya-0.tt.testing"),
		Path:   fmt.Sprintf("/config/%s", "master"),
	}).String()
	fmt.Println(u)
}

[wking]

@staebler, looks like this function approach was yours in #206. Do you remember why you took this route? It seems complicated for a string pointer, vs:

source := fmt.Sprintf("https://api.%s:22623/config/%s", installConfig.ClusterDomain(), role)

[staebler]

I don't think that was authored by me. That was authored by @crawford.

[ajeddeloh]

Would it be possible to add validation (via github.com/coreos/ignition/config/validate.ValidateWithoutSource()) to the final result in this PR. Would be a good check to ensure we're not hitting weird cases.

[wking]

Would it be possible to add validation...

I'm comfortable with e2e suites in CI to cover installer-generated Ignition content. That leaves users who supply their own Ignition files as part of a multi-step install? I'm ok handling that in a separate PR.

[eparis]

/hold
As this PR (in conjunction with others) would break the upgrades we are promising customers after beta3 it may not merge.

[cgwalters]

As this PR (in conjunction with others) would break the upgrades we are promising customers after beta3 it may not merge.

We put a good amount of work into this and think we can pull it off fairly quickly.

[ashcrow]

As this PR (in conjunction with others) would break the upgrades we are promising customers after beta3 it may not merge.

We put a good amount of work into this and think we can pull it off fairly quickly.

Agreed.

[wking]

I realize this is on hold, but when we revive it we need to bump the AWS UPI template too.

[openshift-ci-robot]

@runcom: PR needs rebase.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[openshift-ci-robot]

@runcom: The following tests failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
ci/prow/unit	7b47364	link	/test unit
ci/prow/e2e-aws	7b47364	link	/test e2e-aws
ci/prow/e2e-aws-upgrade	7b47364	link	/test e2e-aws-upgrade

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

[abhinavdahiya]

This looks abandoned. Please feel free to reopen if you think this is still something you would like to pursue

/close

[openshift-ci-robot]

@abhinavdahiya: Closed this PR.

Details

In response to this:

This looks abandoned. Please feel free to reopen if you think this is still something you would like to pursue

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.