Skip to content

Let payload provider render feature gate yaml #2664

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

Merged
openshift-merge-robot merged 1 commit into from
Jun 14, 2023
Merged

Let payload provider render feature gate yaml #2664

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 26 additions & 10 deletions control-plane-operator/controllers/hostedcontrolplane/ignitionserver/ignitionserver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,19 @@ func reconcileDeployment(deployment *appsv1.Deployment,
EmptyDir: &corev1.EmptyDirVolumeSource{},
},
},
{
Name: "bootstrap-manifests",
VolumeSource: corev1.VolumeSource{
EmptyDir: &corev1.EmptyDirVolumeSource{},
},
},
{
Name: "manifests",
VolumeSource: corev1.VolumeSource{
EmptyDir: &corev1.EmptyDirVolumeSource{},
},
},

{
Name: "shared",
VolumeSource: corev1.VolumeSource{
Expand All @@ -437,7 +450,7 @@ func reconcileDeployment(deployment *appsv1.Deployment,
},
Args: []string{
"-c",
invokeFeatureGateRenderScript("/shared", releaseVersion, string(featureGateYAML)),
invokeFeatureGateRenderScript("/shared", string(featureGateYAML)),
Copy link
Contributor

@csrwng csrwng Jun 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If all this container is doing is writing out the feature gate yaml, we can likely do without it. The local ignition provider could read the hcp directly and create the yaml.

Copy link
Member Author

@enxebre enxebre Jun 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, I'm wanting to keep this PR scope to the minimum.
In follow up we can also consider to let the NodePool controller store the featureGate yaml into the token secret so then we just pass it through the getPayload signature.

},
Resources: corev1.ResourceRequirements{
Requests: corev1.ResourceList{
Expand Down Expand Up @@ -522,6 +535,14 @@ func reconcileDeployment(deployment *appsv1.Deployment,
Name: "payloads",
MountPath: "/payloads",
},
{
Name: "bootstrap-manifests",
MountPath: "/usr/share/bootkube/manifests/bootstrap-manifests",
},
{
Name: "manifests",
MountPath: "/usr/share/bootkube/manifests/manifests",
},
{
Name: "shared",
MountPath: "/shared",
Expand Down Expand Up @@ -575,24 +596,19 @@ func reconcilePodMonitor(podMonitor *prometheusoperatorv1.PodMonitor,
return nil
}

func invokeFeatureGateRenderScript(workDir, payloadVersion, featureGateYAML string) string {
// invokeFeatureGateRenderScript writes the passed yaml to disk in a given dir.
func invokeFeatureGateRenderScript(workDir, featureGateYAML string) string {
var script = `#!/bin/bash
set -e
cd /tmp
mkdir input output manifests

touch /tmp/manifests/99_feature-gate.yaml
cat <<EOF >/tmp/manifests/99_feature-gate.yaml
%[3]s
%[2]s
EOF

/usr/bin/cluster-config-operator render \
--config-output-file config \
--asset-input-dir /tmp/input \
--asset-output-dir /tmp/output \
--rendered-manifest-files=/tmp/manifests \
--payload-version=%[2]s
cp /tmp/manifests/99_feature-gate.yaml %[1]s/99_feature-gate.yaml
`
return fmt.Sprintf(script, workDir, payloadVersion, featureGateYAML)
return fmt.Sprintf(script, workDir, featureGateYAML)
}
111 changes: 94 additions & 17 deletions ignition-server/controllers/local_ignitionprovider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,23 +240,6 @@ func (p *LocalIgnitionProvider) GetPayload(ctx context.Context, releaseImage str
return nil, fmt.Errorf("failed to extract templates from image: %w", err)
}

// Write out the feature gate manifest to the MCC dir.
// Use the feature gate from the hosted control plane which should reflect the feature gate of the cluster.
if err := func() error {
featureGateBytes, err := os.ReadFile(p.FeatureGateManifest)
if err != nil {
return fmt.Errorf("failed to read feature gate: %w", err)
}

if err := os.WriteFile(filepath.Join(mccBaseDir, "99_feature-gate.yaml"), featureGateBytes, 0644); err != nil {
return fmt.Errorf("failed to write feature gate: %w", err)
}

return nil
}(); err != nil {
return nil, fmt.Errorf("failed to extract feature gate: %w", err)
}

// Extract binaries from the MCO image into the bin directory
err = func() error {
start := time.Now()
Expand All @@ -276,6 +259,28 @@ func (p *LocalIgnitionProvider) GetPayload(ctx context.Context, releaseImage str
return fmt.Errorf("failed to close file: %w", err)
}
}

component := "cluster-config-operator"
clusterConfigOperatorImage, ok := imageProvider.ImageExist(component)
if !ok {
return fmt.Errorf("release image does not contain cluster-config-operator (images: %v)", imageProvider.ComponentImages())
}
log.Info("discovered cluster config operator image", "image", clusterConfigOperatorImage)

file, err := os.Create(filepath.Join(binDir, component))
if err != nil {
return fmt.Errorf("failed to create file: %w", err)
}
if err := file.Chmod(0777); err != nil {
return fmt.Errorf("failed to chmod file: %w", err)
}
if err := p.ImageFileCache.extractImageFile(ctx, clusterConfigOperatorImage, pullSecret, filepath.Join("usr/bin/", component), file); err != nil {
return fmt.Errorf("failed to extract image file: %w", err)
}
if err := file.Close(); err != nil {
return fmt.Errorf("failed to close file: %w", err)
}

log.Info("downloaded binaries", "time", time.Since(start).Round(time.Second).String())
return nil
}()
Expand All @@ -288,6 +293,31 @@ func (p *LocalIgnitionProvider) GetPayload(ctx context.Context, releaseImage str
return nil, fmt.Errorf("failed to parse payload version: %w", err)
}

featureGateBytes, err := os.ReadFile(p.FeatureGateManifest)
if err != nil {
return nil, fmt.Errorf("failed to read feature gate: %w", err)
}

err = func() error {
start := time.Now()

args := []string{
"-c",
invokeFeatureGateRenderScript(filepath.Join(binDir, "cluster-config-operator"), filepath.Join(workDir, "cco"), mccBaseDir, payloadVersion, string(featureGateBytes)),
}

cmd := exec.CommandContext(ctx, "/bin/bash", args...)
out, err := cmd.CombinedOutput()
log.Info("cluster-config-operator process completed", "time", time.Since(start).Round(time.Second).String(), "output", string(out))
if err != nil {
return fmt.Errorf("cluster-config-operator process failed: %s: %w", string(out), err)
}
return nil
}()
if err != nil {
return nil, fmt.Errorf("failed to execute cluster-config-operator: %w", err)
}

// First, run the MCO using templates and image refs as input. This generates
// output for the MCC.
err = func() error {
Expand Down Expand Up @@ -678,3 +708,50 @@ func findMatchingManifest(imageRef string, deserializedManifestList *manifestlis

return "", fmt.Errorf("imageRef is an unknown format to parse, imageRef: %s", imageRef)
}

func invokeFeatureGateRenderScript(binary, workDir, outputDir string, payloadVersion semver.Version, featureGateYAML string) string {
var script = `#!/bin/bash
set -e
mkdir -p %[2]s

cd %[2]s
mkdir -p input output manifests

touch %[2]s/manifests/99_feature-gate.yaml
cat <<EOF >%[2]s/manifests/99_feature-gate.yaml
%[5]s
EOF

%[1]s render \
--config-output-file config \
--asset-input-dir %[2]s/input \
--asset-output-dir %[2]s/output \
--rendered-manifest-files=%[2]s/manifests \
--payload-version=%[4]s
cp %[2]s/manifests/99_feature-gate.yaml %[3]s/99_feature-gate.yaml
`

// Depending on the version, we need different args.
if payloadVersion.Minor < 14 {
script = `#!/bin/bash
set -e
mkdir -p %[2]s

cd %[2]s
mkdir -p input output manifests

touch %[2]s/manifests/99_feature-gate.yaml
cat <<EOF >%[2]s/manifests/99_feature-gate.yaml
%[5]s
EOF

%[1]s render \
--config-output-file config \
--asset-input-dir %[2]s/input \
--asset-output-dir %[2]s/output
Copy link
Contributor

@muraee muraee Jun 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't we need to pass --payload-version here ?

Copy link
Member Author

@enxebre enxebre Jun 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, only for >= 4.14

cp %[2]s/manifests/99_feature-gate.yaml %[3]s/99_feature-gate.yaml
`
}

return fmt.Sprintf(script, binary, workDir, outputDir, payloadVersion, featureGateYAML)
}
9 changes: 4 additions & 5 deletions test/e2e/nodepool_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,11 +89,10 @@ func TestNodePool(t *testing.T) {
manifestBuilder: NewNTOMachineConfigInPlaceRolloutTestManifest(hostedCluster),
},
*/
// Until this is fixed https://github.com/openshift/hypershift/pull/2664.
//{
// name: "TestNodePoolReplaceUpgrade",
// test: NewNodePoolUpgradeTest(ctx, mgmtClient, hostedCluster, hostedClusterClient, clusterOpts, globalOpts.PreviousReleaseImage, globalOpts.LatestReleaseImage),
//},
{
name: "TestNodePoolReplaceUpgrade",
test: NewNodePoolUpgradeTest(ctx, mgmtClient, hostedCluster, hostedClusterClient, clusterOpts, globalOpts.PreviousReleaseImage, globalOpts.LatestReleaseImage),
},
// TODO: (jparrill) Re-enable when https://issues.redhat.com/browse/OCPBUGS-10218 is fixed
/*
{
Expand Down