Skip to content

Conversation

@djoshy
Copy link
Contributor

@djoshy djoshy commented Jul 11, 2024

- What I did

  • Add support for paths in file based NodeDisruptionPolicies. If multiple policies apply to a file being updated, the closest match will be considered.
  • Moved some of the "backdoor" directory based exceptions into the MCO's default policies.
  • Added a new default policy for the user CA cert to maintain feature parity until it moves to cert_writer.
  • Updated docs to reflect new defaults and path support.

- How to verify it

  • All previous NodeDisruptionPolicy behavior should still behave the same(node disruption e2e should pass)
  • The daemon should now behave correctly for user CA cert changes instead of rebooting.
  • To test the multi-path based policies:
  1. Define more than one file based policy:
apiVersion: operator.openshift.io/v1
kind: MachineConfiguration
metadata:
  name: cluster
  namespace: openshift-machine-config-operator
spec:
  nodeDisruptionPolicy:
    files:
      - path: /etc/example/test
        actions:
          - type: None
      - path: /etc/example/
        actions:
          - reload:
              serviceName: crio.service
            type: Reload
  1. Create an MC targeting the infra/worker pool that adds a file under /etc/example. Observe the daemon logs, this should cause a CRIO reload.
apiVersion: machineconfiguration.openshift.io/v1
kind: MachineConfig
metadata:
  labels:
    machineconfiguration.openshift.io/role: infra
  name:  111-test-file
spec:
  config:
    ignition:
      version: 3.2.0
    storage:
      files:
      - contents:
          source: data:text/plain;charset=utf-8;base64,SSBhbSB2ZXJzaW9uIDEKasas
        path: /etc/example/file
        user:
          name: core
  1. Now create another MC targeting the same pool that adds a file under /etc/example/test. Observe the daemon logs, this should not cause any action.
apiVersion: machineconfiguration.openshift.io/v1
kind: MachineConfig
metadata:
  labels:
    machineconfiguration.openshift.io/role: infra
  name:  112-test-file
spec:
  config:
    ignition:
      version: 3.2.0
    storage:
      files:
      - contents:
          source: data:text/plain;charset=utf-8;base64,SSBhbSB2ZXJzaW9uIDEKasas
        path: /etc/example/test/file
        user:
          name: core

Note: Be sure to add a new MC, and not to overwrite the previous one from step (2). If you overwrite the first test MC, it will cause both policies to be in effect, as you are removing the old test file.

  1. That's it! Nice job! 🚀

@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 11, 2024
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Jul 11, 2024

@djoshy: This pull request references MCO-1125 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.17.0" version, but no target version was set.

Details

In response to this:

- What I did

  • Add support for paths in file based NodeDisruptionPolicies. If multiple paths apply to a a path, the closest match will be considered.
  • Moved some of the "backdoor" directory based exceptions into the MCO's default policies.
  • Updated docs to reflect new defaults and di

- How to verify it

  • All previous NodeDisruptionPolicy behavior should still behave the same(node disruption e2e should pass)
  • To test the multi-path based policies:
  1. Define more than one file based policy:
apiVersion: operator.openshift.io/v1
kind: MachineConfiguration
metadata:
 name: cluster
 namespace: openshift-machine-config-operator
spec:
 nodeDisruptionPolicy:
   files:
     - path: /etc/example/test
       actions:
         - type: None
     - path: /etc/example/
       actions:
         - reload:
             serviceName: crio.service
           type: Reload
  1. Create an MC targeting the infra/worker pool that adds a file under /etc/example. Observe the daemon logs, this should cause a CRIO reload.
apiVersion: machineconfiguration.openshift.io/v1
kind: MachineConfig
metadata:
 labels:
   machineconfiguration.openshift.io/role: infra
 name:  111-test-file
spec:
 config:
   ignition:
     version: 3.2.0
   storage:
     files:
     - contents:
         source: data:text/plain;charset=utf-8;base64,SSBhbSB2ZXJzaW9uIDEKasas
       path: /etc/example/file
       user:
         name: core
  1. Now create another MC targeting the same pool that adds a file under /etc/example/test. Observe the daemon logs, this should not cause any action.
apiVersion: machineconfiguration.openshift.io/v1
kind: MachineConfig
metadata:
 labels:
   machineconfiguration.openshift.io/role: infra
 name:  112-test-file
spec:
 config:
   ignition:
     version: 3.2.0
   storage:
     files:
     - contents:
         source: data:text/plain;charset=utf-8;base64,SSBhbSB2ZXJzaW9uIDEKasas
       path: /etc/example/test/file
       user:
         name: core

Note: Be sure to add a new MC, and not to overwrite the previous one from step (2). If you overwrite the first test MC, it will cause both policies to be in effect, as you are removing the old test file.

  1. That's it! Nice job! 🚀

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 openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 11, 2024
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Jul 11, 2024

@djoshy: This pull request references MCO-1125 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.17.0" version, but no target version was set.

Details

In response to this:

- What I did

  • Add support for paths in file based NodeDisruptionPolicies. If multiple paths apply to a a path, the closest match will be considered.
  • Moved some of the "backdoor" directory based exceptions into the MCO's default policies.
  • Updated docs to reflect new defaults and path support.

- How to verify it

  • All previous NodeDisruptionPolicy behavior should still behave the same(node disruption e2e should pass)
  • To test the multi-path based policies:
  1. Define more than one file based policy:
apiVersion: operator.openshift.io/v1
kind: MachineConfiguration
metadata:
 name: cluster
 namespace: openshift-machine-config-operator
spec:
 nodeDisruptionPolicy:
   files:
     - path: /etc/example/test
       actions:
         - type: None
     - path: /etc/example/
       actions:
         - reload:
             serviceName: crio.service
           type: Reload
  1. Create an MC targeting the infra/worker pool that adds a file under /etc/example. Observe the daemon logs, this should cause a CRIO reload.
apiVersion: machineconfiguration.openshift.io/v1
kind: MachineConfig
metadata:
 labels:
   machineconfiguration.openshift.io/role: infra
 name:  111-test-file
spec:
 config:
   ignition:
     version: 3.2.0
   storage:
     files:
     - contents:
         source: data:text/plain;charset=utf-8;base64,SSBhbSB2ZXJzaW9uIDEKasas
       path: /etc/example/file
       user:
         name: core
  1. Now create another MC targeting the same pool that adds a file under /etc/example/test. Observe the daemon logs, this should not cause any action.
apiVersion: machineconfiguration.openshift.io/v1
kind: MachineConfig
metadata:
 labels:
   machineconfiguration.openshift.io/role: infra
 name:  112-test-file
spec:
 config:
   ignition:
     version: 3.2.0
   storage:
     files:
     - contents:
         source: data:text/plain;charset=utf-8;base64,SSBhbSB2ZXJzaW9uIDEKasas
       path: /etc/example/test/file
       user:
         name: core

Note: Be sure to add a new MC, and not to overwrite the previous one from step (2). If you overwrite the first test MC, it will cause both policies to be in effect, as you are removing the old test file.

  1. That's it! Nice job! 🚀

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.

@djoshy djoshy force-pushed the node-disrupt-path branch from 08f3826 to 72cfc73 Compare July 16, 2024 14:42
@djoshy djoshy changed the title MCO-1125: Allow paths to be defined for non-disruptive updates MCO-1125: OCPBUGS-35277: Allow paths to be defined for non-disruptive updates Jul 16, 2024
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Jul 16, 2024

@djoshy: This pull request references MCO-1125 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.17.0" version, but no target version was set.

Details

In response to this:

- What I did

  • Add support for paths in file based NodeDisruptionPolicies. If multiple paths apply to a a path, the closest match will be considered.
  • Moved some of the "backdoor" directory based exceptions into the MCO's default policies.
  • Added a new default policy for the user CA cert to maintain feature parity until it moves to cert_writer.
  • Updated docs to reflect new defaults and path support.

- How to verify it

  • All previous NodeDisruptionPolicy behavior should still behave the same(node disruption e2e should pass)
  • The daemon should now behave correctly for user CA cert changes instead of rebooting.
  • To test the multi-path based policies:
  1. Define more than one file based policy:
apiVersion: operator.openshift.io/v1
kind: MachineConfiguration
metadata:
 name: cluster
 namespace: openshift-machine-config-operator
spec:
 nodeDisruptionPolicy:
   files:
     - path: /etc/example/test
       actions:
         - type: None
     - path: /etc/example/
       actions:
         - reload:
             serviceName: crio.service
           type: Reload
  1. Create an MC targeting the infra/worker pool that adds a file under /etc/example. Observe the daemon logs, this should cause a CRIO reload.
apiVersion: machineconfiguration.openshift.io/v1
kind: MachineConfig
metadata:
 labels:
   machineconfiguration.openshift.io/role: infra
 name:  111-test-file
spec:
 config:
   ignition:
     version: 3.2.0
   storage:
     files:
     - contents:
         source: data:text/plain;charset=utf-8;base64,SSBhbSB2ZXJzaW9uIDEKasas
       path: /etc/example/file
       user:
         name: core
  1. Now create another MC targeting the same pool that adds a file under /etc/example/test. Observe the daemon logs, this should not cause any action.
apiVersion: machineconfiguration.openshift.io/v1
kind: MachineConfig
metadata:
 labels:
   machineconfiguration.openshift.io/role: infra
 name:  112-test-file
spec:
 config:
   ignition:
     version: 3.2.0
   storage:
     files:
     - contents:
         source: data:text/plain;charset=utf-8;base64,SSBhbSB2ZXJzaW9uIDEKasas
       path: /etc/example/test/file
       user:
         name: core

Note: Be sure to add a new MC, and not to overwrite the previous one from step (2). If you overwrite the first test MC, it will cause both policies to be in effect, as you are removing the old test file.

  1. That's it! Nice job! 🚀

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.

@djoshy djoshy force-pushed the node-disrupt-path branch from 72cfc73 to 59a0955 Compare July 16, 2024 14:48
@sergiordlr
Copy link
Contributor

sergiordlr commented Jul 19, 2024

Verified OCPBUGS-35277 using IPI on AWS

Test cases passed with enabled techpreview:

"[sig-mco] MCO security Author:rioliu-NonHyperShiftHOST-NonPreRelease-Longduration-High-71991-post action of user-ca-bundle change will skip drain,reboot and restart crio service [Disruptive] [Serial]"
"[sig-mco] MCO security Author:sregidor-NonHyperShiftHOST-High-67660-MCS generates ignition configs with certs [Disruptive] [Serial]"

@openshift-ci openshift-ci bot added the qe-approved Signifies that QE has signed off on this PR label Jul 19, 2024
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Jul 19, 2024

@djoshy: This pull request references MCO-1125 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.17.0" version, but no target version was set.

Details

In response to this:

- What I did

  • Add support for paths in file based NodeDisruptionPolicies. If multiple paths apply to a a path, the closest match will be considered.
  • Moved some of the "backdoor" directory based exceptions into the MCO's default policies.
  • Added a new default policy for the user CA cert to maintain feature parity until it moves to cert_writer.
  • Updated docs to reflect new defaults and path support.

- How to verify it

  • All previous NodeDisruptionPolicy behavior should still behave the same(node disruption e2e should pass)
  • The daemon should now behave correctly for user CA cert changes instead of rebooting.
  • To test the multi-path based policies:
  1. Define more than one file based policy:
apiVersion: operator.openshift.io/v1
kind: MachineConfiguration
metadata:
 name: cluster
 namespace: openshift-machine-config-operator
spec:
 nodeDisruptionPolicy:
   files:
     - path: /etc/example/test
       actions:
         - type: None
     - path: /etc/example/
       actions:
         - reload:
             serviceName: crio.service
           type: Reload
  1. Create an MC targeting the infra/worker pool that adds a file under /etc/example. Observe the daemon logs, this should cause a CRIO reload.
apiVersion: machineconfiguration.openshift.io/v1
kind: MachineConfig
metadata:
 labels:
   machineconfiguration.openshift.io/role: infra
 name:  111-test-file
spec:
 config:
   ignition:
     version: 3.2.0
   storage:
     files:
     - contents:
         source: data:text/plain;charset=utf-8;base64,SSBhbSB2ZXJzaW9uIDEKasas
       path: /etc/example/file
       user:
         name: core
  1. Now create another MC targeting the same pool that adds a file under /etc/example/test. Observe the daemon logs, this should not cause any action.
apiVersion: machineconfiguration.openshift.io/v1
kind: MachineConfig
metadata:
 labels:
   machineconfiguration.openshift.io/role: infra
 name:  112-test-file
spec:
 config:
   ignition:
     version: 3.2.0
   storage:
     files:
     - contents:
         source: data:text/plain;charset=utf-8;base64,SSBhbSB2ZXJzaW9uIDEKasas
       path: /etc/example/test/file
       user:
         name: core

Note: Be sure to add a new MC, and not to overwrite the previous one from step (2). If you overwrite the first test MC, it will cause both policies to be in effect, as you are removing the old test file.

  1. That's it! Nice job! 🚀

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.

@sergiordlr
Copy link
Contributor

/remove-label qe-approved

@openshift-ci openshift-ci bot removed the qe-approved Signifies that QE has signed off on this PR label Jul 19, 2024
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Jul 19, 2024

@djoshy: This pull request references MCO-1125 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.17.0" version, but no target version was set.

Details

In response to this:

- What I did

  • Add support for paths in file based NodeDisruptionPolicies. If multiple paths apply to a a path, the closest match will be considered.
  • Moved some of the "backdoor" directory based exceptions into the MCO's default policies.
  • Added a new default policy for the user CA cert to maintain feature parity until it moves to cert_writer.
  • Updated docs to reflect new defaults and path support.

- How to verify it

  • All previous NodeDisruptionPolicy behavior should still behave the same(node disruption e2e should pass)
  • The daemon should now behave correctly for user CA cert changes instead of rebooting.
  • To test the multi-path based policies:
  1. Define more than one file based policy:
apiVersion: operator.openshift.io/v1
kind: MachineConfiguration
metadata:
 name: cluster
 namespace: openshift-machine-config-operator
spec:
 nodeDisruptionPolicy:
   files:
     - path: /etc/example/test
       actions:
         - type: None
     - path: /etc/example/
       actions:
         - reload:
             serviceName: crio.service
           type: Reload
  1. Create an MC targeting the infra/worker pool that adds a file under /etc/example. Observe the daemon logs, this should cause a CRIO reload.
apiVersion: machineconfiguration.openshift.io/v1
kind: MachineConfig
metadata:
 labels:
   machineconfiguration.openshift.io/role: infra
 name:  111-test-file
spec:
 config:
   ignition:
     version: 3.2.0
   storage:
     files:
     - contents:
         source: data:text/plain;charset=utf-8;base64,SSBhbSB2ZXJzaW9uIDEKasas
       path: /etc/example/file
       user:
         name: core
  1. Now create another MC targeting the same pool that adds a file under /etc/example/test. Observe the daemon logs, this should not cause any action.
apiVersion: machineconfiguration.openshift.io/v1
kind: MachineConfig
metadata:
 labels:
   machineconfiguration.openshift.io/role: infra
 name:  112-test-file
spec:
 config:
   ignition:
     version: 3.2.0
   storage:
     files:
     - contents:
         source: data:text/plain;charset=utf-8;base64,SSBhbSB2ZXJzaW9uIDEKasas
       path: /etc/example/test/file
       user:
         name: core

Note: Be sure to add a new MC, and not to overwrite the previous one from step (2). If you overwrite the first test MC, it will cause both policies to be in effect, as you are removing the old test file.

  1. That's it! Nice job! 🚀

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.

@sergiordlr
Copy link
Contributor

/hold

Still verifying the path functionality

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 19, 2024
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Jul 19, 2024

@djoshy: This pull request references MCO-1125 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.17.0" version, but no target version was set.

Details

In response to this:

- What I did

  • Add support for paths in file based NodeDisruptionPolicies. If multiple policies apply to a file being updated, the closest match will be considered.
  • Moved some of the "backdoor" directory based exceptions into the MCO's default policies.
  • Added a new default policy for the user CA cert to maintain feature parity until it moves to cert_writer.
  • Updated docs to reflect new defaults and path support.

- How to verify it

  • All previous NodeDisruptionPolicy behavior should still behave the same(node disruption e2e should pass)
  • The daemon should now behave correctly for user CA cert changes instead of rebooting.
  • To test the multi-path based policies:
  1. Define more than one file based policy:
apiVersion: operator.openshift.io/v1
kind: MachineConfiguration
metadata:
 name: cluster
 namespace: openshift-machine-config-operator
spec:
 nodeDisruptionPolicy:
   files:
     - path: /etc/example/test
       actions:
         - type: None
     - path: /etc/example/
       actions:
         - reload:
             serviceName: crio.service
           type: Reload
  1. Create an MC targeting the infra/worker pool that adds a file under /etc/example. Observe the daemon logs, this should cause a CRIO reload.
apiVersion: machineconfiguration.openshift.io/v1
kind: MachineConfig
metadata:
 labels:
   machineconfiguration.openshift.io/role: infra
 name:  111-test-file
spec:
 config:
   ignition:
     version: 3.2.0
   storage:
     files:
     - contents:
         source: data:text/plain;charset=utf-8;base64,SSBhbSB2ZXJzaW9uIDEKasas
       path: /etc/example/file
       user:
         name: core
  1. Now create another MC targeting the same pool that adds a file under /etc/example/test. Observe the daemon logs, this should not cause any action.
apiVersion: machineconfiguration.openshift.io/v1
kind: MachineConfig
metadata:
 labels:
   machineconfiguration.openshift.io/role: infra
 name:  112-test-file
spec:
 config:
   ignition:
     version: 3.2.0
   storage:
     files:
     - contents:
         source: data:text/plain;charset=utf-8;base64,SSBhbSB2ZXJzaW9uIDEKasas
       path: /etc/example/test/file
       user:
         name: core

Note: Be sure to add a new MC, and not to overwrite the previous one from step (2). If you overwrite the first test MC, it will cause both policies to be in effect, as you are removing the old test file.

  1. That's it! Nice job! 🚀

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.

@sergiordlr
Copy link
Contributor

sergiordlr commented Jul 19, 2024

Verified path support using IPI on AWS

Configure 3 paths in the Machineconfiguration nodedisruption section

  • (dir) /etc/test-file-policy-subdir-75109/extradir/
    action none
  • (dir) /etc/test-file-policy-subdir-75109
    action restart crio
  • (file) /etc/test-file-policy-subdir-75109/test-file.txt
    action reload crio

1 Create a MC for /etc/test-file-policy-subdir-75109/test-file.txt
2 Check that crio.service was reloaded
3 Create a MC for /etc/test-file-policy-subdir-75109/test-dir.txt
4 Check that crio.service was restarted
5 Create a MC for /etc/test-file-policy-subdir-75109/extradir/test-file.txt
6 Check that no action is executed

The commands for every step can be found in test case: OCP-75109 - NodeDisruptionPolicy files allow paths to be defined for non-disruptive updates

/label qe-approved

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 19, 2024

@sergiordlr: Those labels are not set on the issue: qe-approved

Details

In response to this:

Verified path support using IPI on AWS

Configure 3 paths in the Machineconfiguration nodedisruption section

  • (dir) /etc/test-file-policy-subdir-75109/extradir/
    action none
  • (dir) /etc/test-file-policy-subdir-75109
    action restart crio
  • (file) /etc/test-file-policy-subdir-75109/test-file.txt
    action reload crio

1 Create a MC for /etc/test-file-policy-subdir-75109/test-file.txt
2 Check that crio.service was reloaded
3 Create a MC for /etc/test-file-policy-subdir-75109/test-dir.txt
4 Check that crio.service was restarted
5 Create a MC for /etc/test-file-policy-subdir-75109/extradir/test-file.txt
6 Check that no action is executed

The commands for every step can be found in test case: OCP-75109 - NodeDisruptionPolicy files allow paths to be defined for non-disruptive updates

/remove-label qe-approved

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.

@sergiordlr
Copy link
Contributor

/label qe-approved

@openshift-ci openshift-ci bot added the qe-approved Signifies that QE has signed off on this PR label Jul 19, 2024
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Jul 19, 2024

@djoshy: This pull request references MCO-1125 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.17.0" version, but no target version was set.

Details

In response to this:

- What I did

  • Add support for paths in file based NodeDisruptionPolicies. If multiple policies apply to a file being updated, the closest match will be considered.
  • Moved some of the "backdoor" directory based exceptions into the MCO's default policies.
  • Added a new default policy for the user CA cert to maintain feature parity until it moves to cert_writer.
  • Updated docs to reflect new defaults and path support.

- How to verify it

  • All previous NodeDisruptionPolicy behavior should still behave the same(node disruption e2e should pass)
  • The daemon should now behave correctly for user CA cert changes instead of rebooting.
  • To test the multi-path based policies:
  1. Define more than one file based policy:
apiVersion: operator.openshift.io/v1
kind: MachineConfiguration
metadata:
 name: cluster
 namespace: openshift-machine-config-operator
spec:
 nodeDisruptionPolicy:
   files:
     - path: /etc/example/test
       actions:
         - type: None
     - path: /etc/example/
       actions:
         - reload:
             serviceName: crio.service
           type: Reload
  1. Create an MC targeting the infra/worker pool that adds a file under /etc/example. Observe the daemon logs, this should cause a CRIO reload.
apiVersion: machineconfiguration.openshift.io/v1
kind: MachineConfig
metadata:
 labels:
   machineconfiguration.openshift.io/role: infra
 name:  111-test-file
spec:
 config:
   ignition:
     version: 3.2.0
   storage:
     files:
     - contents:
         source: data:text/plain;charset=utf-8;base64,SSBhbSB2ZXJzaW9uIDEKasas
       path: /etc/example/file
       user:
         name: core
  1. Now create another MC targeting the same pool that adds a file under /etc/example/test. Observe the daemon logs, this should not cause any action.
apiVersion: machineconfiguration.openshift.io/v1
kind: MachineConfig
metadata:
 labels:
   machineconfiguration.openshift.io/role: infra
 name:  112-test-file
spec:
 config:
   ignition:
     version: 3.2.0
   storage:
     files:
     - contents:
         source: data:text/plain;charset=utf-8;base64,SSBhbSB2ZXJzaW9uIDEKasas
       path: /etc/example/test/file
       user:
         name: core

Note: Be sure to add a new MC, and not to overwrite the previous one from step (2). If you overwrite the first test MC, it will cause both policies to be in effect, as you are removing the old test file.

  1. That's it! Nice job! 🚀

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.

Copy link
Contributor

@yuqi-zhang yuqi-zhang left a comment

Choose a reason for hiding this comment

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

General logic makes sense to me! I think 2 things here:

  1. let's add some additional unit testing around FindClosestFilePolicyPathMatch. I don't think there should be any edge cases but best to be safe, especially if you have multiple policies
  2. let's make sure we capture this change in other documentation as well (maybe enhancement? for sure the openshift docs)

@djoshy
Copy link
Contributor Author

djoshy commented Jul 22, 2024

let's add some additional unit testing around FindClosestFilePolicyPathMatch. I don't think there should be any edge cases but best to be safe, especially if you have multiple policies

Added a few cases I could think of, let me know if that works!

let's make sure we capture this change in other documentation as well (maybe enhancement? for sure the openshift docs)

I'll make a card in the epic to track this (:

@djoshy
Copy link
Contributor Author

djoshy commented Jul 22, 2024

/retest-required

Copy link
Contributor

@yuqi-zhang yuqi-zhang left a comment

Choose a reason for hiding this comment

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

/lgtm

Tests lgtm, thanks!

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 23, 2024
@djoshy djoshy force-pushed the node-disrupt-path branch from b5d9002 to 9d52686 Compare July 23, 2024 12:43
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jul 23, 2024
@djoshy
Copy link
Contributor Author

djoshy commented Jul 23, 2024

/unhold

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 23, 2024
@yuqi-zhang
Copy link
Contributor

/lgtm

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

openshift-ci bot commented Jul 23, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: djoshy, yuqi-zhang

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

@djoshy
Copy link
Contributor Author

djoshy commented Jul 23, 2024

/test e2e-gcp-op-techpreview

@djoshy
Copy link
Contributor Author

djoshy commented Jul 23, 2024

/test e2e-gcp-op
/test e2e-gcp-op-techpreview

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 23, 2024

@djoshy: 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-vsphere-ovn-upi 9d52686 link false /test e2e-vsphere-ovn-upi
ci/prow/e2e-vsphere-ovn-zones 9d52686 link false /test e2e-vsphere-ovn-zones
ci/prow/e2e-vsphere-ovn-upi-zones 9d52686 link false /test e2e-vsphere-ovn-upi-zones

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.

@openshift-merge-bot openshift-merge-bot bot merged commit 3acda20 into openshift:master Jul 23, 2024
@openshift-bot
Copy link
Contributor

[ART PR BUILD NOTIFIER]

Distgit: ose-machine-config-operator
This PR has been included in build ose-machine-config-operator-container-v4.18.0-202407232010.p0.g3acda20.assembly.stream.el9.
All builds following this will include this PR.

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/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. qe-approved Signifies that QE has signed off on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants