diff --git a/src/DIRAC/WorkloadManagementSystem/Agent/PilotLoggingAgent.py b/src/DIRAC/WorkloadManagementSystem/Agent/PilotLoggingAgent.py index fbf2ae36d97..e25f40c47d6 100644 --- a/src/DIRAC/WorkloadManagementSystem/Agent/PilotLoggingAgent.py +++ b/src/DIRAC/WorkloadManagementSystem/Agent/PilotLoggingAgent.py @@ -46,7 +46,7 @@ def initialize(self): """ # pilot logs lifetime in days self.clearPilotsDelay = self.am_getOption("ClearPilotsDelay", self.clearPilotsDelay) - # configured VOs and setup + # configured VOs res = getVOs() if not res["OK"]: return res @@ -64,7 +64,6 @@ def initialize(self): if pilotLogging: res = opsHelper.getOptionsDict("Shifter/DataManager") if not res["OK"]: - # voRes[vo] = "No shifter defined - skipped" self.log.error(f"No shifter defined for VO: {vo} - skipping ...") continue @@ -84,6 +83,7 @@ def initialize(self): continue userDNs = result["Value"] # a same user may have more than one DN fd, filename = tempfile.mkstemp(prefix=vo + "__") + print("filename", filename) os.close(fd) vomsAttr = getVOMSAttributeForGroup(proxyGroup) result = getProxy(userDNs, proxyGroup, vomsAttr=vomsAttr, proxyFilePath=filename) @@ -92,7 +92,7 @@ def initialize(self): self.log.error( f"Could not download a proxy for DN {userDNs}, group {proxyGroup} for VO {vo}, skipped" ) - return result + continue self.proxyDict[vo] = result["Value"] return S_OK() diff --git a/src/DIRAC/WorkloadManagementSystem/Agent/test/Test_Agent_PilotLoggingAgent.py b/src/DIRAC/WorkloadManagementSystem/Agent/test/Test_Agent_PilotLoggingAgent.py index 2e7939e48b6..82760e70d7b 100644 --- a/src/DIRAC/WorkloadManagementSystem/Agent/test/Test_Agent_PilotLoggingAgent.py +++ b/src/DIRAC/WorkloadManagementSystem/Agent/test/Test_Agent_PilotLoggingAgent.py @@ -17,6 +17,9 @@ # Mock Objects mockReply = MagicMock() mockReply1 = MagicMock() +mockGetDNForUsername = MagicMock() +mockGetVomsAttr = MagicMock() +mockGetProxy = MagicMock() mockOperations = MagicMock() mockTornadoClient = MagicMock() mockDataManager = MagicMock() @@ -48,19 +51,16 @@ def plaBase(mocker): mocker.patch( "DIRAC.WorkloadManagementSystem.Agent.PilotLoggingAgent.Operations.getOptionsDict", side_effect=mockReply1 ) + mocker.patch( + "DIRAC.WorkloadManagementSystem.Agent.PilotLoggingAgent.getDNForUsername", side_effect=mockGetDNForUsername + ) + mocker.patch("DIRAC.WorkloadManagementSystem.Agent.PilotLoggingAgent.getProxy", side_effect=mockGetProxy) pla = PilotLoggingAgent() pla.log = gLogger pla._AgentModule__configDefaults = mockAM return pla -@pytest.fixture -def pla_initialised(mocker, plaBase): - mocker.patch("DIRAC.WorkloadManagementSystem.Agent.PilotLoggingAgent.PilotLoggingAgent.executeForVO") - plaBase.initialize() - return plaBase - - @pytest.fixture def pla(mocker, plaBase): mocker.patch( @@ -72,42 +72,60 @@ def pla(mocker, plaBase): "DIRAC.WorkloadManagementSystem.Agent.PilotLoggingAgent.DataManager", side_effect=mockDataManager, ) - plaBase.initialize() return plaBase -def test_initialize(plaBase): +@pytest.mark.parametrize( + "remoteLogging, options, getDN, getVOMS, getProxy, resDict, expectedRes", + [ + ([True, False], upDict, S_OK(["myDN"]), S_OK(), S_OK("proxyfilename"), {"gridpp": "proxyfilename"}, S_OK()), + ([False, False], upDict, S_OK(["myDN"]), S_OK(), S_OK(), {}, S_OK()), + ([True, False], upDict, S_ERROR("Could not obtain a DN"), S_OK(), S_OK(), {}, S_OK()), + ([True, False], upDict, S_ERROR("Could not download proxy"), S_OK(), S_ERROR("Failure"), {}, S_OK()), + ], +) +def test_initialize(plaBase, remoteLogging, options, getDN, getVOMS, getProxy, resDict, expectedRes): + """ + After a successful initialisation the proxyDict should contain proxy filenames key by a VO name. + test loops: gridpp enabled, lz disabled, proxy obtained, result proxyDict contains a proxy filename. + both VOs disabled => proxyDict empty + gridpp enabled, by getDNForUsername fails => proxyDict empty + gridpp enabled, lz disabled, getProxy fails => proxyDict empty + + """ + mockReply.side_effect = remoteLogging # Operations.getValue("/Pilot/RemoteLogging", False) + mockReply1.return_value = options # Operations.getOptionsDict("Shifter/DataManager") + mockGetDNForUsername.return_value = getDN + mockGetVomsAttr.return_value = getVOMS + mockGetProxy.return_value = getProxy + res = plaBase.initialize() + assert plaBase.voList == plaModule.getVOs()["Value"] - assert res == S_OK() + assert resDict == plaBase.proxyDict + assert res == expectedRes @pytest.mark.parametrize( - "mockReplyInput, expected, expectedExecOut, expected2", + "proxyDict, execVORes, expectedResult", [ - ("/Pilot/RemoteLogging", [True, False], S_OK(), upDict), - ("/Pilot/RemoteLogging", [False, False], S_OK(), upDict), - ("/Pilot/RemoteLogging", [True, False], S_ERROR("Execute for VO failed"), upDict), + ({}, S_OK(), S_OK()), + ({"gridpp": "gridpp_proxyfile"}, S_OK(), S_OK()), + ( + {"gridpp": "gridpp_proxyfile"}, + S_ERROR("Execute for VO failed"), + S_ERROR("Agent cycle for some VO finished with errors"), + ), ], ) -def test_execute(pla_initialised, mockReplyInput, expected, expectedExecOut, expected2): +def test_execute(plaBase, proxyDict, execVORes, expectedResult): """Testing a thin version of execute (executeForVO is mocked)""" - assert pla_initialised.voList == plaModule.getVOs()["Value"] - mockReply.side_effect = expected - mockReply1.return_value = expected2 - # remote pilot logging on (gridpp only) and off. - pla_initialised.executeForVO.return_value = expectedExecOut - res = pla_initialised.execute() - if not any(expected): - pla_initialised.executeForVO.assert_not_called() - else: - assert pla_initialised.executeForVO.called - pla_initialised.executeForVO.assert_called_with( - "gridpp", - proxyUserName=upDict["Value"]["User"], - proxyUserGroup=upDict["Value"]["Group"], - ) - assert res["OK"] == expectedExecOut["OK"] + + plaBase.executeForVO = MagicMock() + plaBase.proxyDict = proxyDict + plaBase.executeForVO.return_value = execVORes + res = plaBase.execute() + assert res["OK"] == expectedResult["OK"] @pytest.mark.parametrize(