Skip to content
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.

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

OCPBUGS-16380: Add /etc/containers volume on create-cluster-and-infraenv #7332

Merged

Conversation

danielerez
Copy link
Contributor

@danielerez danielerez commented Jul 16, 2023

As done in assisted-service.service.template, added '-v /etc/containers:/etc/containers' when HaveMirrorConfig flag is true to the create-cluster-and-infraenv service. This should allow propagation of registries.conf, which is required on disconnected environments when using OPENSHIFT_INSTALL_RELEASE_IMAGE_MIRROR.

As done in assisted-service.service.template,
added '-v /etc/containers:/etc/containers' when HaveMirrorConfig
flag is true rtp create-cluster-and-infraenv service.

This should allow propagation of registries.conf, which is requried
on disconnected environments.
@danielerez
Copy link
Contributor Author

/cc @bfournie
/cc @andfasano
/cc @rwsu

danielerez added a commit to danielerez/appliance that referenced this pull request Jul 17, 2023
The create-cluster-and-infraenv service fails on disconnected
using 4.14.0-ec.3 release[*]

Hence, overridden the service and added '-v /etc/containers:/etc/containers'
for propagating the registries.conf file.

Notes:
* A proper [fix](openshift/installer#7332) pushed to the installer for later.
* It won't affect connect environments as the registries.conf file doesn't have mirrors config by default.

[*] create-cluster-and-infraenv[2784]: level=fatal msg="Failed to register cluster with assisted-service: command 'oc adm release info -o template --template '{{.metadata.version}}' --insecure=true quay.io/openshift-release-dev/ocp-release@sha256:3c050cb52fdd3e65c518d4999d238ec026ef724503f275377fee6bf0d33093ab --registry-config=/tmp/registry-config1560177852' exited with non-zero exit code 1: \nerror: unable to read image quay.io/openshift-release-dev/ocp-release@sha256:3c050cb52fdd3e65c518d4999d238ec026ef724503f275377fee6bf0d33093ab: Get \"http://quay.io/v2/\": net/http: request canceled while waiting for connection (Client.Timeout exceeded while awaiting headers)\n"
openshift-merge-robot pushed a commit to openshift/appliance that referenced this pull request Jul 17, 2023
The create-cluster-and-infraenv service fails on disconnected
using 4.14.0-ec.3 release[*]

Hence, overridden the service and added '-v /etc/containers:/etc/containers'
for propagating the registries.conf file.

Notes:
* A proper [fix](openshift/installer#7332) pushed to the installer for later.
* It won't affect connect environments as the registries.conf file doesn't have mirrors config by default.

[*] create-cluster-and-infraenv[2784]: level=fatal msg="Failed to register cluster with assisted-service: command 'oc adm release info -o template --template '{{.metadata.version}}' --insecure=true quay.io/openshift-release-dev/ocp-release@sha256:3c050cb52fdd3e65c518d4999d238ec026ef724503f275377fee6bf0d33093ab --registry-config=/tmp/registry-config1560177852' exited with non-zero exit code 1: \nerror: unable to read image quay.io/openshift-release-dev/ocp-release@sha256:3c050cb52fdd3e65c518d4999d238ec026ef724503f275377fee6bf0d33093ab: Get \"http://quay.io/v2/\": net/http: request canceled while waiting for connection (Client.Timeout exceeded while awaiting headers)\n"
@rwsu
Copy link
Contributor

rwsu commented Jul 17, 2023

/retest-required

1 similar comment
@danielerez
Copy link
Contributor Author

/retest-required

@bfournie
Copy link
Contributor

Is the same change needed for apply-host-config.service which also uses agent-installer-client?
https://github.com/openshift/installer/blob/master/data/data/agent/systemd/units/apply-host-config.service#L17

@danielerez
Copy link
Contributor Author

Is the same change needed for apply-host-config.service which also uses agent-installer-client? https://github.com/openshift/installer/blob/master/data/data/agent/systemd/units/apply-host-config.service#L17

Seems it's unnecessary there as we don't pass the OPENSHIFT_INSTALL_RELEASE_IMAGE_MIRROR env var.

@bfournie
Copy link
Contributor

The change looks good but is it possible to create a bug to describe what this is fixing? We haven't seen this omission causing a problem with the general agent installer flow (yet). Was the issue specific to the Openshift appliance?

@danielerez
Copy link
Contributor Author

danielerez commented Jul 18, 2023

The change looks good but is it possible to create a bug to describe what this is fixing? We haven't seen this omission causing a problem with the general agent installer flow (yet). Was the issue specific to the Openshift appliance?

I think the issue is specifically for when passing the OPENSHIFT_INSTALL_RELEASE_IMAGE_MIRROR var on disconnected env. So I guess this specific use-case isn't tested on dev-scripts. But sure, I can open a ticket for it if you think it makes sense.

@danielerez
Copy link
Contributor Author

The change looks good but is it possible to create a bug to describe what this is fixing? We haven't seen this omission causing a problem with the general agent installer flow (yet). Was the issue specific to the Openshift appliance?

I think the issue is specifically for when passing the OPENSHIFT_INSTALL_RELEASE_IMAGE_MIRROR var on disconnected env. So I guess this specific use-case isn't tested on dev-scripts. But sure, I can open a ticket for it if you think it makes sense.

Opened a ticket: https://issues.redhat.com/browse/OCPBUGS-16380

@danielerez danielerez changed the title Add /etc/containers volume on create-cluster-and-infraenv OCPBUGS-16380: Add /etc/containers volume on create-cluster-and-infraenv Jul 19, 2023
@openshift-ci-robot openshift-ci-robot added jira/severity-moderate Referenced Jira bug's severity is moderate for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. labels Jul 19, 2023
@openshift-ci-robot
Copy link
Contributor

@danielerez: This pull request references Jira Issue OCPBUGS-16380, which is invalid:

  • expected the bug to target the "4.14.0" version, but no target version was set

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

As done in assisted-service.service.template, added '-v /etc/containers:/etc/containers' when HaveMirrorConfig flag is true to the create-cluster-and-infraenv service. This should allow propagation of registries.conf, which is required on disconnected environments when using OPENSHIFT_INSTALL_RELEASE_IMAGE_MIRROR.

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 openshift-ci-robot added the jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. label Jul 19, 2023
@bfournie
Copy link
Contributor

/jira refresh

@openshift-ci-robot openshift-ci-robot added jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. and removed jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. labels Jul 19, 2023
@openshift-ci-robot
Copy link
Contributor

@bfournie: This pull request references Jira Issue OCPBUGS-16380, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (4.14.0) matches configured target version for branch (4.14.0)
  • bug is in the state New, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @mhanss

In response to this:

/jira refresh

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 openshift-ci bot requested a review from mhanss July 19, 2023 21:48
@bfournie
Copy link
Contributor

/approve

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 21, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bfournie

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

The pull request process is described here

Needs approval from an approver in each of these files:

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 21, 2023
Copy link
Contributor

@rwsu rwsu left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 24, 2023
@rwsu
Copy link
Contributor

rwsu commented Jul 24, 2023

/retest-required

@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD 3c0c45c and 2 for PR HEAD 92ab8b5 in total

@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD bd102ca and 1 for PR HEAD 92ab8b5 in total

@danielerez
Copy link
Contributor Author

/retest-required

@danielerez
Copy link
Contributor Author

/test okd-scos-images

@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD 8cd8f79 and 0 for PR HEAD 92ab8b5 in total

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 26, 2023

@danielerez: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-agent-ha-dualstack 92ab8b5 link false /test e2e-agent-ha-dualstack
ci/prow/e2e-agent-sno-ipv4-pxe 92ab8b5 link false /test e2e-agent-sno-ipv4-pxe

Full PR test history. Your PR dashboard.

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.

@danielerez
Copy link
Contributor Author

/test e2e-agent-compact-ipv4

@openshift-merge-robot openshift-merge-robot merged commit 26a0d5c into openshift:master Jul 26, 2023
23 of 25 checks passed
@openshift-ci-robot
Copy link
Contributor

@danielerez: Jira Issue OCPBUGS-16380: All pull requests linked via external trackers have merged:

Jira Issue OCPBUGS-16380 has been moved to the MODIFIED state.

In response to this:

As done in assisted-service.service.template, added '-v /etc/containers:/etc/containers' when HaveMirrorConfig flag is true to the create-cluster-and-infraenv service. This should allow propagation of registries.conf, which is required on disconnected environments when using OPENSHIFT_INSTALL_RELEASE_IMAGE_MIRROR.

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.

danielerez added a commit to danielerez/appliance that referenced this pull request Aug 6, 2023
Includes the following fixes:
* Static and DHCP networking should work
* Messages should be displayed to console (with agetty)
* When 'UserCorePass' isn't specified, the installer sets
  the core pass on load-config-iso to 'auth/kubeadmin-password'.

The following changes were made to support the new ignition:
* Extracted 'podman run' script of registry the service
  * Required for supporting static networking
    (as values are loaded from config image).
* Used REQUIRED_MASTER_NODES/REQUIRED_WORKER_NODES env vars
  in update-hosts script.
* Removed create-cluster-and-infraenv service
  (as already added in: openshift/installer#7332)
openshift-merge-robot pushed a commit to openshift/appliance that referenced this pull request Aug 6, 2023
Includes the following fixes:
* Static and DHCP networking should work
* Messages should be displayed to console (with agetty)
* When 'UserCorePass' isn't specified, the installer sets
  the core pass on load-config-iso to 'auth/kubeadmin-password'.

The following changes were made to support the new ignition:
* Extracted 'podman run' script of registry the service
  * Required for supporting static networking
    (as values are loaded from config image).
* Used REQUIRED_MASTER_NODES/REQUIRED_WORKER_NODES env vars
  in update-hosts script.
* Removed create-cluster-and-infraenv service
  (as already added in: openshift/installer#7332)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/severity-moderate Referenced Jira bug's severity is moderate for the branch this PR is targeting. jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants