Skip to content

HIVE-2548: Run installmanager binary in install container for fips compatibility#2260

Merged
2uasimojo merged 3 commits intoopenshift:mce-2.6from
lleshchi:HIVE-2400
Jun 21, 2024
Merged

HIVE-2548: Run installmanager binary in install container for fips compatibility#2260
2uasimojo merged 3 commits intoopenshift:mce-2.6from
lleshchi:HIVE-2400

Conversation

@lleshchi
Copy link
Contributor

@lleshchi lleshchi commented Apr 17, 2024

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 17, 2024
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Apr 17, 2024

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@lleshchi
Copy link
Contributor Author

/test all

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 17, 2024
@codecov
Copy link

codecov bot commented Apr 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 58.55%. Comparing base (cd191be) to head (d1629e3).
Report is 2 commits behind head on mce-2.6.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##           mce-2.6    #2260   +/-   ##
========================================
  Coverage    58.54%   58.55%           
========================================
  Files          182      182           
  Lines        25830    25829    -1     
========================================
+ Hits         15123    15124    +1     
+ Misses        9431     9429    -2     
  Partials      1276     1276           
Files Coverage Δ
contrib/pkg/utils/generic.go 11.21% <ø> (+0.40%) ⬆️
pkg/imageset/updateinstaller.go 45.06% <100.00%> (-0.67%) ⬇️
pkg/install/generate.go 47.62% <100.00%> (+0.48%) ⬆️

... and 1 file with indirect coverage changes

@lleshchi
Copy link
Contributor Author

/test all

@lleshchi
Copy link
Contributor Author

/test all

1 similar comment
@lleshchi
Copy link
Contributor Author

/test all

@lleshchi
Copy link
Contributor Author

/retest e2e

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Apr 19, 2024

@lleshchi: The /retest command does not accept any targets.
The following commands are available to trigger required jobs:

  • /test coverage
  • /test e2e
  • /test e2e-azure
  • /test e2e-gcp
  • /test e2e-pool
  • /test images
  • /test periodic-images
  • /test security
  • /test unit
  • /test verify

Use /test all to run the following jobs that were automatically triggered:

  • pull-ci-openshift-hive-master-coverage
  • pull-ci-openshift-hive-master-e2e
  • pull-ci-openshift-hive-master-e2e-pool
  • pull-ci-openshift-hive-master-images
  • pull-ci-openshift-hive-master-periodic-images
  • pull-ci-openshift-hive-master-security
  • pull-ci-openshift-hive-master-unit
  • pull-ci-openshift-hive-master-verify
Details

In response to this:

/retest e2e

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.

@lleshchi
Copy link
Contributor Author

/test e2e

1 similar comment
@lleshchi
Copy link
Contributor Author

/test e2e

@lleshchi lleshchi force-pushed the HIVE-2400 branch 3 times, most recently from 2c3d29f to f581d13 Compare May 21, 2024 21:00
@lleshchi
Copy link
Contributor Author

/test all

@lleshchi lleshchi changed the title test run hiveutil from installer container Run installmanager binary in install container for fips compatibility May 21, 2024
@lleshchi lleshchi marked this pull request as ready for review May 22, 2024 14:25
@lleshchi
Copy link
Contributor Author

/assign @2uasimojo

@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 22, 2024
@openshift-ci openshift-ci bot requested review from dlom and jstuever May 22, 2024 14:43
Copy link
Member

@2uasimojo 2uasimojo left a comment

Choose a reason for hiding this comment

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

I have substantive concerns over cert installation. We should ask QE for a pre-merge run that specifically targets that path.

@2uasimojo
Copy link
Member

I have substantive concerns over cert installation.

These concerns have been assuaged.

But...

We should ask QE for a pre-merge run that specifically targets that path.

...we should still totally do this. From our conversation:

I think we want to make sure QE hits e.g. this path by making sure CERTS_SECRET_NAME is set, which comes from here if this returns a cert secret ref, which was set via CertificatesSecretRef.
The test case would be like:
Make sure your vsphere env is set up to require additional certs, and run an install without ⬆️ to make sure it fails. Then run with it and make sure it works.

Copy link
Member

@2uasimojo 2uasimojo left a comment

Choose a reason for hiding this comment

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

I think this is ready to go
/lgtm

but
/hold
cause we want to do some pre-merge QE.

lleshchi added 2 commits June 18, 2024 11:35
As a result of the openshift installer transitioning from rhel8 to rhel9
(openshift/installer#8196), running
openshift-install in the rhel8 backed hive container in order to install
a cluster in fips mode results in a fips incompatibility.

Create a seperate installmanager binary that runs the install-manager
command previously invoked by hiveutil. Build a rhel8 and rhel9 version
of hive, and copy both versions of installmanager to the installer
container. The directory struture of the provisioning pod is also
adjusted to support this change. Lastly, the installmanager binary
corresponding to the rhel version of the installer container.

Signed-off-by: Leah Leshchinsky <lleshchi@redhat.com>
Signed-off-by: Leah Leshchinsky <lleshchi@redhat.com>
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 18, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 2uasimojo, lleshchi

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:

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

@2uasimojo
Copy link
Member

/retest

1 similar comment
@celebdor
Copy link
Contributor

/retest

@2uasimojo
Copy link
Member

/test e2e

@2uasimojo
Copy link
Member

/test e2e e2e-pool

Looks like new permission requirements were added:

time="2024-06-20T14:26:11Z" level=warning msg="Action not allowed with tested creds" action="elasticloadbalancing:SetSecurityGroups"
time="2024-06-20T14:26:11Z" level=warning msg="Action not allowed with tested creds" action="s3:PutBucketPolicy"

I've added these to the ci user. Trying again.

@2uasimojo
Copy link
Member

security fail: #2308 needs to be backported. Overriding for now.

/override ci/prow/security

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 20, 2024

@2uasimojo: Overrode contexts on behalf of 2uasimojo: ci/prow/security

Details

In response to this:

security fail: #2308 needs to be backported. Overriding for now.

/override ci/prow/security

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-sigs/prow repository.

@2uasimojo
Copy link
Member

/test e2e e2e-pool

I... must not have hit the final "go" button on those permissions?

@2uasimojo
Copy link
Member

Same error, but now I know for sure the perms are there. WTF?

/test e2e e2e-pool

@2uasimojo
Copy link
Member

/test e2e-pool

infra flake

@2uasimojo
Copy link
Member

I forgot again that our CI user is in a different AWS account.

/test e2e e2e-pool

@2uasimojo
Copy link
Member

/test e2e-pool

1 similar comment
@2uasimojo
Copy link
Member

/test e2e-pool

@2uasimojo
Copy link
Member

/test e2e-pool

same infra flake -- opening DPTP request

@celebdor
Copy link
Contributor

celebdor commented Jun 21, 2024

/retitle HIVE-2548: Run installmanager binary in install container for fips compatibility

@openshift-ci openshift-ci bot changed the title HIVE-2400: Run installmanager binary in install container for fips compatibility HIVE-2548: Run installmanager binary in install container for fips compatibility Jun 21, 2024
@openshift-ci-robot
Copy link

openshift-ci-robot commented Jun 21, 2024

@lleshchi: This pull request references HIVE-2548 which is a valid jira issue.

Details

In response to this:

HIVE-2400

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 openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci-robot
Copy link

openshift-ci-robot commented Jun 21, 2024

@lleshchi: This pull request references HIVE-2548 which is a valid jira issue.

Details

In response to this:

HIVE-2400
HIVE-2548

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 openshift-eng/jira-lifecycle-plugin repository.

1 similar comment
@openshift-ci-robot
Copy link

openshift-ci-robot commented Jun 21, 2024

@lleshchi: This pull request references HIVE-2548 which is a valid jira issue.

Details

In response to this:

HIVE-2400
HIVE-2548

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 openshift-eng/jira-lifecycle-plugin repository.

@2uasimojo
Copy link
Member

/test e2e-pool

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 21, 2024

@lleshchi: 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-azure 674956a link true /test e2e-azure
ci/prow/security d1629e3 link true /test security
ci/prow/e2e-pool d1629e3 link true /test e2e-pool

Full PR test history. Your PR dashboard.

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-sigs/prow repository. I understand the commands that are listed here.

@2uasimojo 2uasimojo merged commit 0ed7370 into openshift:mce-2.6 Jun 21, 2024
2uasimojo added a commit to 2uasimojo/hive that referenced this pull request Jun 21, 2024
In openshift#2260 we changed how we're invoking the openshift-install binary.

Before: Copy openshift-install into the hive container and run it via
`/usr/bin/hiveutil install-manager`

After: Copy hiveutil into the installer container and run installer via
`/output/hiveutil.rhel$VER install-manager`

What we missed was that, for STS flows, we inject an AWS credentials
file containing a `credential_process` that invoked `/usr/bin/hiveutil
install-manager aws-credentials` -- but `hiveutil` no longer lives
there.

Fix.

HIVE-2400
2uasimojo added a commit to 2uasimojo/hive that referenced this pull request Jun 21, 2024
In openshift#2260 we changed how we're invoking the openshift-install binary.

Before: Copy openshift-install into the hive container and run it via
`/usr/bin/hiveutil install-manager`

After: Copy hiveutil into the installer container and run installer via
`/output/hiveutil.rhel$VER install-manager`

What we missed was that, for STS flows, we inject an AWS credentials
file containing a `credential_process` that invoked `/usr/bin/hiveutil
install-manager aws-credentials` -- but `hiveutil` no longer lives
there.

Fix.

HIVE-2400
openshift-cherrypick-robot pushed a commit to openshift-cherrypick-robot/hive that referenced this pull request Jun 21, 2024
In openshift#2260 we changed how we're invoking the openshift-install binary.

Before: Copy openshift-install into the hive container and run it via
`/usr/bin/hiveutil install-manager`

After: Copy hiveutil into the installer container and run installer via
`/output/hiveutil.rhel$VER install-manager`

What we missed was that, for STS flows, we inject an AWS credentials
file containing a `credential_process` that invoked `/usr/bin/hiveutil
install-manager aws-credentials` -- but `hiveutil` no longer lives
there.

Fix.

HIVE-2400
celebdor pushed a commit to celebdor/hive that referenced this pull request Jun 25, 2024
In openshift#2260 we changed how we're invoking the openshift-install binary.

Before: Copy openshift-install into the hive container and run it via
`/usr/bin/hiveutil install-manager`

After: Copy hiveutil into the installer container and run installer via
`/output/hiveutil.rhel$VER install-manager`

What we missed was that, for STS flows, we inject an AWS credentials
file containing a `credential_process` that invoked `/usr/bin/hiveutil
install-manager aws-credentials` -- but `hiveutil` no longer lives
there.

Fix.

HIVE-2400
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. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants