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

MGMT-15264: Block installation if no custom manifests in OCI cluster #5364

Conversation

eliorerz
Copy link
Contributor

/cc @eliorerz

List all the issues related to this PR

  • New Feature
  • Enhancement
  • Bug fix
  • Tests
  • Documentation
  • CI/CD

What environments does this code impact?

  • Automation (CI, tools, etc)
  • Cloud
  • Operator Managed Deployments
  • None

How was this code tested?

  • assisted-test-infra environment
  • dev-scripts environment
  • Reviewer's test appreciated
  • Waiting for CI to do a full test run
  • Manual (Elaborate on how it was tested)
  • No tests needed

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jul 17, 2023
@openshift-ci-robot
Copy link

openshift-ci-robot commented Jul 17, 2023

@eliorerz: This pull request references MGMT-15264 which is a valid jira issue.

In response to this:

/cc @eliorerz

List all the issues related to this PR

  • New Feature
  • Enhancement
  • Bug fix
  • Tests
  • Documentation
  • CI/CD

What environments does this code impact?

  • Automation (CI, tools, etc)
  • Cloud
  • Operator Managed Deployments
  • None

How was this code tested?

  • assisted-test-infra environment
  • dev-scripts environment
  • Reviewer's test appreciated
  • Waiting for CI to do a full test run
  • Manual (Elaborate on how it was tested)
  • No tests needed

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
Copy link

openshift-ci bot commented Jul 17, 2023

@eliorerz: GitHub didn't allow me to request PR reviews from the following users: eliorerz.

Note that only openshift members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @eliorerz

List all the issues related to this PR

  • New Feature
  • Enhancement
  • Bug fix
  • Tests
  • Documentation
  • CI/CD

What environments does this code impact?

  • Automation (CI, tools, etc)
  • Cloud
  • Operator Managed Deployments
  • None

How was this code tested?

  • assisted-test-infra environment
  • dev-scripts environment
  • Reviewer's test appreciated
  • Waiting for CI to do a full test run
  • Manual (Elaborate on how it was tested)
  • No tests needed

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 added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. api-review Categorizes an issue or PR as actively needing an API review. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jul 17, 2023
@codecov
Copy link

codecov bot commented Jul 17, 2023

Codecov Report

Merging #5364 (c6978a0) into master (c4869af) will increase coverage by 0.18%.
Report is 13 commits behind head on master.
The diff coverage is 80.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5364      +/-   ##
==========================================
+ Coverage   67.53%   67.71%   +0.18%     
==========================================
  Files         226      226              
  Lines       33293    33616     +323     
==========================================
+ Hits        22485    22764     +279     
- Misses       8774     8811      +37     
- Partials     2034     2041       +7     
Files Changed Coverage Δ
internal/cluster/validator.go 96.06% <72.72%> (-0.88%) ⬇️
internal/cluster/refresh_status_preprocessor.go 94.32% <100.00%> (+0.12%) ⬆️
internal/cluster/validation_id.go 92.30% <100.00%> (ø)

... and 9 files with indirect coverage changes

@eliorerz
Copy link
Contributor Author

/retest

swagger.yaml Outdated Show resolved Hide resolved
internal/cluster/validator.go Outdated Show resolved Hide resolved
internal/cluster/validator.go Outdated Show resolved Hide resolved
internal/cluster/validator.go Outdated Show resolved Hide resolved
@eliorerz eliorerz force-pushed the MGMT-15264_Block_installation_if_no_custom_manifests_in_OCI_cluster_as_installation_will_fail branch from 0a8e8a9 to 88f483b Compare July 19, 2023 11:21
Copy link
Contributor

@gamli75 gamli75 left a comment

Choose a reason for hiding this comment

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

I don't remember if we have a specific code but we should not run this validation during the installation

internal/cluster/validator.go Outdated Show resolved Hide resolved
return ValidationSuccess, "Platform requirements satisfied"
}

usages, err := usage.Unmarshal(c.cluster.Cluster.FeatureUsage)
Copy link
Contributor

Choose a reason for hiding this comment

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

why we are using the feature usage for checking it and not data from the cluster? what will happen if there will be a bug in the feature usage?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There isn't a bug thanks to @avishayt #5363 :)
But the alternative is adding a boolean read only parameter to the cluster

internal/cluster/validator.go Show resolved Hide resolved
@eliorerz eliorerz force-pushed the MGMT-15264_Block_installation_if_no_custom_manifests_in_OCI_cluster_as_installation_will_fail branch from 88f483b to d9e1d3e Compare July 19, 2023 13:01
@adriengentil
Copy link
Contributor

/test e2e-oci-assisted

@openshift-ci
Copy link

openshift-ci bot commented Jul 20, 2023

@adriengentil: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

  • /test e2e-agent-compact-ipv4
  • /test edge-assisted-operator-catalog-publish-verify
  • /test edge-ci-index
  • /test edge-e2e-ai-operator-ztp
  • /test edge-e2e-ai-operator-ztp-sno-day2-workers
  • /test edge-e2e-ai-operator-ztp-sno-day2-workers-late-binding
  • /test edge-e2e-metal-assisted
  • /test edge-e2e-metal-assisted-4-10
  • /test edge-e2e-metal-assisted-4-11
  • /test edge-e2e-metal-assisted-4-12
  • /test edge-e2e-metal-assisted-4-9
  • /test edge-e2e-metal-assisted-cnv
  • /test edge-e2e-metal-assisted-lvm
  • /test edge-e2e-metal-assisted-odf
  • /test edge-images
  • /test edge-lint
  • /test edge-subsystem-aws
  • /test edge-subsystem-kubeapi-aws
  • /test edge-unit-test
  • /test edge-verify-generated-code
  • /test images
  • /test mce-images

The following commands are available to trigger optional jobs:

  • /test e2e-agent-ha-dualstack
  • /test e2e-agent-sno-ipv6
  • /test edge-e2e-ai-operator-ztp-3masters
  • /test edge-e2e-ai-operator-ztp-capi
  • /test edge-e2e-ai-operator-ztp-compact-day2-masters
  • /test edge-e2e-ai-operator-ztp-compact-day2-workers
  • /test edge-e2e-ai-operator-ztp-disconnected
  • /test edge-e2e-ai-operator-ztp-hypershift-zero-nodes
  • /test edge-e2e-ai-operator-ztp-multiarch-3masters-ocp-411
  • /test edge-e2e-ai-operator-ztp-multiarch-sno-ocp-411
  • /test edge-e2e-ai-operator-ztp-node-labels
  • /test edge-e2e-ai-operator-ztp-sno-day2-masters
  • /test edge-e2e-ai-operator-ztp-sno-day2-workers-ignitionoverride
  • /test edge-e2e-metal-assisted-4-13
  • /test edge-e2e-metal-assisted-4-14
  • /test edge-e2e-metal-assisted-day2
  • /test edge-e2e-metal-assisted-day2-arm-workers
  • /test edge-e2e-metal-assisted-day2-single-node
  • /test edge-e2e-metal-assisted-ipv4v6
  • /test edge-e2e-metal-assisted-ipv6
  • /test edge-e2e-metal-assisted-kube-api-late-binding-single-node
  • /test edge-e2e-metal-assisted-kube-api-late-unbinding-ipv4-single-node
  • /test edge-e2e-metal-assisted-kube-api-net-suite
  • /test edge-e2e-metal-assisted-mce
  • /test edge-e2e-metal-assisted-mce-4-10
  • /test edge-e2e-metal-assisted-mce-sno
  • /test edge-e2e-metal-assisted-metallb
  • /test edge-e2e-metal-assisted-none
  • /test edge-e2e-metal-assisted-onprem
  • /test edge-e2e-metal-assisted-single-node
  • /test edge-e2e-metal-assisted-static-ip-suite
  • /test edge-e2e-metal-assisted-tang
  • /test edge-e2e-metal-assisted-tpmv2
  • /test edge-e2e-metal-assisted-upgrade-agent
  • /test edge-e2e-nutanix-assisted
  • /test edge-e2e-nutanix-assisted-2workers
  • /test edge-e2e-oci-assisted
  • /test edge-e2e-vsphere-assisted
  • /test edge-e2e-vsphere-assisted-4-12
  • /test edge-e2e-vsphere-assisted-4-13
  • /test edge-e2e-vsphere-assisted-umn
  • /test edge-push-pr-image
  • /test push-pr-image

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

  • pull-ci-openshift-assisted-service-master-e2e-agent-compact-ipv4
  • pull-ci-openshift-assisted-service-master-edge-ci-index
  • pull-ci-openshift-assisted-service-master-edge-e2e-ai-operator-ztp
  • pull-ci-openshift-assisted-service-master-edge-e2e-ai-operator-ztp-capi
  • pull-ci-openshift-assisted-service-master-edge-e2e-ai-operator-ztp-hypershift-zero-nodes
  • pull-ci-openshift-assisted-service-master-edge-e2e-metal-assisted
  • pull-ci-openshift-assisted-service-master-edge-images
  • pull-ci-openshift-assisted-service-master-edge-lint
  • pull-ci-openshift-assisted-service-master-edge-subsystem-aws
  • pull-ci-openshift-assisted-service-master-edge-subsystem-kubeapi-aws
  • pull-ci-openshift-assisted-service-master-edge-unit-test
  • pull-ci-openshift-assisted-service-master-edge-verify-generated-code
  • pull-ci-openshift-assisted-service-master-images
  • pull-ci-openshift-assisted-service-master-mce-images

In response to this:

/test e2e-oci-assisted

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.

@adriengentil
Copy link
Contributor

/test edge-e2e-oci-assisted

@eliorerz eliorerz force-pushed the MGMT-15264_Block_installation_if_no_custom_manifests_in_OCI_cluster_as_installation_will_fail branch from d9e1d3e to 8037876 Compare July 25, 2023 13:42
…chine-set operator would not go degraded due to placeholder credentials on installation.

Update prism central and prism element address placeholder, so it won't be confusing on post installation steps.
@eliorerz eliorerz force-pushed the MGMT-15264_Block_installation_if_no_custom_manifests_in_OCI_cluster_as_installation_will_fail branch from 8037876 to c6978a0 Compare July 25, 2023 14:22
@eliorerz
Copy link
Contributor Author

/test edge-e2e-oci-assisted

@gamli75
Copy link
Contributor

gamli75 commented Jul 26, 2023

I don't remember if we have a specific code but we should not run this validation during the installation

@eliorerz did you check it?

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

openshift-ci bot commented Jul 26, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: eliorerz, gamli75

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

@eliorerz
Copy link
Contributor Author

/override ci/prow/edge-e2e-ai-operator-ztp-capi

@openshift-ci
Copy link

openshift-ci bot commented Jul 26, 2023

@eliorerz: Overrode contexts on behalf of eliorerz: ci/prow/edge-e2e-ai-operator-ztp-capi

In response to this:

/override ci/prow/edge-e2e-ai-operator-ztp-capi

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
Copy link

openshift-ci bot commented Jul 26, 2023

@eliorerz: all tests passed!

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.

@openshift-merge-robot openshift-merge-robot merged commit 255d3db into openshift:master Jul 26, 2023
17 checks passed
@eliorerz eliorerz deleted the MGMT-15264_Block_installation_if_no_custom_manifests_in_OCI_cluster_as_installation_will_fail branch July 26, 2023 19:24
CrystalChun pushed a commit to CrystalChun/assisted-service that referenced this pull request Aug 25, 2023
…chine-set operator would not go degraded due to placeholder credentials on installation. (openshift#5364)

Update prism central and prism element address placeholder, so it won't be confusing on post installation steps.
danielerez pushed a commit to danielerez/assisted-service that referenced this pull request Oct 15, 2023
…chine-set operator would not go degraded due to placeholder credentials on installation. (openshift#5364)

Update prism central and prism element address placeholder, so it won't be confusing on post installation steps.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-review Categorizes an issue or PR as actively needing an API review. approved Indicates a PR has been approved by an approver from all required OWNERS files. 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. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants